Hi Samuel,
El 7/4/26 a les 1:13, Samuel Thibault ha escrit:
> Mmmm, I'm really surprised. When flags is not 0, the ip_cmsg_recv_*
> calls below end up calling put_cmsg which use msg->msg_control and
> msg->msg_controllen, without initializing them first, so it's up to
> callers of ip_cmsg_recv to initialize them? So if you are getting
> garbage, there is some initialization missing indeed, but it looks wrong
> to be adding it here. In principle that would be in the earliest caller,
> i.e. S_io_read, which does initialize the msghdr.
>
> So I don't know where it needs to be fixed, but this changes rather
> looks like hiding a bug than fixing it.
>
Thanks for the review, in fact, there was a pretty big bug here.
The problem is that the `put_cmsg` function does this for each requested field
in the control block:
```c
msg->msg_control += cmlen;
msg->msg_controllen -= cmlen;
```
The mig server stub always sends a 2048 char buffer as `control`, and `2048` as
`controllen`
```
char control[2048];
...
controlP = control;
controlCnt = 2048;
OutP->RetCode = S_socket_recv(sock, &OutP->addr.name, &addrPoly, In0P->flags,
&dataP, &dataCnt, &portsP, &portsPoly, &portsCnt, &controlP, &controlCnt,
&outflags, In0P->amount);
```
`put_cmsg` assumes a `msg_controllen` of `2048` and, for each flag set in the
socket, substracts the corresponding size from `msg_controllen`. Due to that,
this line in pfinet was wrong:
```c
*controllen = m.msg_controllen;
```
It was setting `*controllen` to `2048` when no flags, and `2024` when flag is
`IP_PKTINFO`. But it should set the value to the actual amount of bytes written
in the control block instead. That is, `0` and `24`.
Then the mig server stub does this:
```c
if (controlP != control) {
... /* out-of-line scenario */
}
else {
if (controlCnt) {
memcpy(OutP->control, control, controlCnt);
}
}
```
In the case of `controlCnt == 0` it doesn't copy the control block data back to
the caller. But due to the bug, `controlCnt` was `2048` here and it was
writting garbage.
So the patch is simple, just this:
```c
*controllen -= m.msg_controllen;
```
That fixes everything. The previous patch can be discarded then.
On the other hand, something that really confuses me is that, in the previous
scenario where garbage was being returned to the client, I found that the
garbage was in fact valid control block data from previous calls. Does this
make sense? is the same memory buffer being reused between calls?
