Subject: Re: SCM_RIGHTS broken?
To: der Mouse <mouse@Rodents.Montreal.QC.CA>
From: Greg Troxel <gdt@ir.bbn.com>
List: tech-kern
Date: 02/02/2005 09:30:25
der Mouse <mouse@Rodents.Montreal.QC.CA> writes:

> 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/.)

Sorry, I read it backwards - I had meant to verify only that the
message buffer fit within the control message before reading it.

I suppose one could check for controllen being equal to either
  CMSG_LEN(msg->cmsg_len)
or
  CMSG_SPACE(msg->cmsg_len)

-- other values don't seem legitimate (since I think padding is
specified to be via CMSG_SPACE).

>         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.
>                  */
> 
> 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).

Silently ignoring extra messages doesn't seem like good behavior to me.

> 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.)

I think users are required to use CMSG_SPACE and CMSG_NEXTHDR to
allocate space and find the next header, so I think it does.
I read most of it when fixing quagga's router advertisement code,
which was crashing sparc and sparc64 kernels, since the kernel assumed
padded messages, and I had to make the quagga code work on other
systems, including those that don't define __CMSG_ALIGN.

-- 
        Greg Troxel <gdt@ir.bbn.com>