Subject: Re: SCM_RIGHTS broken?
To: None <tech-kern@NetBSD.org>
From: der Mouse <mouse@Rodents.Montreal.QC.CA>
List: tech-kern
Date: 02/01/2005 20:03:12
>> Specifically, RFC3542 says that msg_controllen may include padding
>> after the last control message (which in our implementation means
>> padding to the boundary determined by __cmsg_alignbytes()).  But if
>> I do this, the test in unp_internalize() that tests for
>> cm->cmsg_len != control->m_len fires, because cm->cmsg_len is the
>> CMSG_LEN() value whereas control->m_len is the CMSG_SPACE() value.

>> It looks to me as though the test in unp_internalize should read
>> cm->cmsg_len > control->m_len to allow for padding.

> It looks like a real bug, if the RFC allows padding at the end (as
> opposed to between).

I'm fairly sure it does (but see below about how 3542 does not directly
bear on SCM_RIGHTS).  See the diagram at the top of page 62, and the
text a few lines below saying

                                                  While sending an
   application may or may not include padding at the end of last
   ancillary data in msg_controllen and implementations must accept both
   as valid.

(It also notes that receiving applications must allow space for padding
in case the kernel wants to provide it, but that's not the issue at
hand.)

> The test should be >=, though,

I don't think so; you do not want to error out if they are equal!
(The test is "if this test fires, error out" - the old code uses !=,
which I think should be replaced with >: error out if the cmsg claims
more bytes than are actually there, rather than, s/more/& or fewer/.)

> and this begs the question whether it is allowable to have a second
> cmsghdr with something else on the send call, rather than just extra
> padding

It's permitted per RFC3542 control-message formatting rules, but we
have never permitted it for SCM_RIGHTS messages.  The AF_LOCAL usrreq
procedure, case PRU_SEND, starts with

        case PRU_SEND:
                /*
                 * Note: unp_internalize() rejects any control message
                 * other than SCM_RIGHTS, and only allows one.  This
                 * has the side-effect of preventing a caller from
                 * forging SCM_CREDS.
                 */

While there is a misplaced "only", it's clear that the "just one cmsg"
semantic is deliberate.  The comment is at least five years old; it's
present in my 1.4T kernel source, and I fuzzily remember something
similar in the 4.3 source back when I was working with 4.3.  Note that
RFC3542 is not concerned with SCM_RIGHTS messages; it's all about IPv6.
(Its effect on SCM_RIGHTS is purely because they both use the same
ancillary-control-data socket interface - which has been a moderately
long-standing annoyance of mine; in 1.4T, SCM_RIGHTS was handled in a
way compatible with the old API, but soon after that, things silently
changed, breaking existing code, when the SCM_RIGHTS code was made to
use the CMSG_* macros.)

My suggested change does mean that multiple cmsgs would be allowed,
with all but the first being ignored as padding.  If this is considered
a Bad Thing, it would, I think, also work to error unless cm->cmsg_len
is either control->m_len (no padding) or CMSG_ALIGN(control->m_len).
It's not clear to me whether the RFC specifies the amount of padding.
The API given cannot work if arbitrary amounts of padding are
permitted, but I do not find a clear statement to that effect, only the
implication implicit in the API design.  (Perhaps I just haven't looked
in the right place; I have not read the whole RFC start to finish.)

> (guessing 4 bytes of padding on sparc or sparc64, where
> __cmsg_alignbytes is 8, I think).

Actually, it's 7, but that's a quibble; you're right that sparc, at
least, uses 8-byte alignment.  (I noticed this on sparc.)

/~\ The ASCII				der Mouse
\ / Ribbon Campaign
 X  Against HTML	       mouse@rodents.montreal.qc.ca
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B