Bug 644003

I wrote:
> The examples use (int *)CMSG_DATA(cmsg).  That can give an
> unaligned int*: cmsg points into a char buffer (...)

Never mind that.  "cmsg->cmsg_len = ..." & co would fail for the
same reason, so CMSG_FIRSTHDR() will have to take care of that by
adjusting the address on hosts that require aligned access.

OTOH the code does get gcc complaints about type aliasing, since
it accesses data via another type than it was declared/created
with. (Accesses char as int, it's the opposite which is allowed.)

I'm hoping someone will someday create a safer interface, but
that's a rather bigger issue than manpage fixes.

This request was evaluated by Red Hat Product Management for inclusion in Red Hat Enterprise Linux 5.7 and Red Hat does not plan to fix this issue the currently developed update.

Contact your manager or support representative in case you need to escalate this bug.

This request was evaluated by Red Hat Product Management for
inclusion in the current release of Red Hat Enterprise Linux.
Because the affected component is not scheduled to be updated in the
current release, Red Hat is unfortunately unable to address this
request at this time. Red Hat invites you to ask your support
representative to propose this request, if appropriate and relevant,
in the next release of Red Hat Enterprise Linux.

This bug wasn't fixed via M-P-O, so changing to the correct component.

Created attachment 587252 [details]
first draft of cmsg(3) man page

Created attachment 587253 [details]
cmsg.patch

Hello Hallvard,

see below my suggestions and comments:

(In reply to comment #0)
> Created attachment 454185 [details]
> Test program for SCM_RIGHTS
> 
> Description of problem:
> 
> man 3 cmsg has several problems.  Possibly some should be referred
> to whoever defined the original CMSG_*(), if anyone does.
> 
> 
> The examples use (int *)CMSG_DATA(cmsg).  That can give an
> unaligned int*: cmsg points into a char buffer - at least in the
> SCM_RIGHTS example, and presumably in both.  They should instead
> pass CMSG_DATA(cmsg) to memcpy().
> 
I am ignoring this issue according to comment 1.
> 
> The SCM_RIGHTS example ends with msg.msg_controllen =
> cmsg->cmsg_len; which should be ... = CMSG_SPACE(sizeof myfds);
> The descriptive text a few paragraphs above gets it right.
> 
> Except this statement can presumably be dropped in this case,
> since it is the same value as we set originally.
> 
> Except in my testing it works to use CMSG_LEN when there's just
> one cmsg, and in recvmsg this works to limit the number of file
> descriptors received.  The latter is useful, see below.  But since
> there's no clear documentation of msg_controllen requrements, I
> guess that counts as an unsupported or undocumented hack.
> 
To fix this, I suggest to remove the last line of that example (msg.msg_controllen = cmsg->cmsg_len;) as it is not necessary there.
> 
> As far as I can tell, neither manpage (cmsg, sendmsg, recvmsg)
> mention that at least one msg_iov byte must be sent and received.
> Except it might be deduced from recvmsg(2) saying return value =
> #bytes received = 0 means socket shutdown.
> The SCM_RIGHTS example in particular looks complete, just missing
> the sendmsg(), but the recvmsg() call will hang without the sender
> iov or fail without the receiver iov.
> 
I suggest to add the sendmsg() call to the SCM_RIGHTS example.
> 
> The cmsg(3) and/or recvmsg(2) manpages should warn that the sender
> can inject a file descriptor leak into a sloppy receiver program:
> If that accepts msg_control but does not parse all control messages,
> checking for and close()ing any unexpected SCM_RIGHTS descriptors.
> 
> However, I don't see an "official" way of doing that:
> 
> First, there's no documented way to know how many descriptors were
> received.  The doc doesn't even say it is possible:  The CMSG_LEN
> doc mentions alignment, so it could round up just like CMSG_SPACE.
> Anyway, it doesn't on Linux, so I've settled for this:
> 
>    enum { cm_offset = CMSG_LEN(sizeof(int)) - sizeof(int) };
>    int num_fd = (int)(cmsg->cmsg_len - cm_offset) / (int)sizeof(int);
> 
> CMSG_LEN(nonzero) in case it meets some OS where it's not written
> to handle zero.  (Assuming several OSes support CMSG_LEN & co and
> do it slightly differently, I don't know.)  The (int) casts are so
> if the above formula breaks and cmsg_len < cm_offset, I get near
> zero and not a huge size_t wraparound.  One could also fill buf[]
> with (int)-1 values first, and check that the resulting num_fd > 0.
> 
To fix this, I would add the following note to the cmsg(3) man page:

The receiver should parse each control message and close every
file descriptor appropriately; otherwise, the sender can inject
a file descriptor leak into the receiver. The number of all
file descriptors passed to the receiver can be computed using the length
of the message, e.g. int nb_of_descriptors = cmsg->cmsg_len / sizeof (int);
in the following examples.
>
> Second, what should the "is this SCM_RIGHTS?" test be?  As general
> as possible in case the sender manages to set some strange values.
> I guess it's best written (cmsg->cmsg_type == SCM_RIGHTS) without
> the additional (cmsg->cmsg_level==... &&) from the IP_TTL example.
> Is that latter test needed?  And is it needed for SCM_RIGHTS?
> 
In my opinion and according to the BSD man page [1], test for SCM_RIGHTS should contain also additional checks for length and level. I don't think that additional checks will break anything, but I really can't tell whether they are required.

I've attached first draft of cmsg(3) man page and the patch showing the difference. Please, do you have any other comments?

Also, I've CC'ed Jeff Law, who helped a lot with these suggestions and Michael Kerrisk, maintainer of man pages who could bring his opinion to these suggestions as well.


[1] http://resin.csoft.net/cgi-bin/man.cgi?section=3&topic=CMSG_DATA

This request was evaluated by Red Hat Product Management for
inclusion in the current release of Red Hat Enterprise Linux.
Because the affected component is not scheduled to be updated in the
current release, Red Hat is unfortunately unable to address this
request at this time. Red Hat invites you to ask your support
representative to propose this request, if appropriate and relevant,
in the next release of Red Hat Enterprise Linux.

Hallvard, are you still interested in this bug report?

ping

Feel free to reopen this ticket if you are still interested in fixing it.