Re: [PATCH 2/3] network: recvmsg and sendmsg standard compliance (BZ#16919)

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 07-04-2016 06:22, Szabolcs Nagy wrote:
> On 28/03/16 15:34, Adhemerval Zanella wrote:
>> POSIX specifies that both msghdr::msg_iovlen and msghdr::msg_controllen
>> to be of size int and socklen_t respectively.  However Linux defines it as
>> both size_t and for 64-bit it requires some adjustments to make the
>> functions standard compliance.
>>
>> This patch fixes it by creating a temporary header and zeroing the pad
>> fields for 64-bits architecture where size of size_t exceeds the size of
>> the int.
> 
> sendmsg is harder to fix because cmsghdr also needs fix ups:
> 
>>  /* Structure used for storage of ancillary data object information.  */
>>  struct cmsghdr
>>    {
>> -    size_t cmsg_len;		/* Length of data in cmsg_data plus length
>> -				   of cmsghdr structure.
>> -				   !! The type should be socklen_t but the
>> -				   definition of the kernel is incompatible
>> -				   with this.  */
>> +#if __BYTE_ORDER == __BIG_ENDIAN
>> +    int __glibc_reserved1;	/* Pad toadjust Linux size to POSIX defined
>> +				   size for cmsg_len.  */
>> +    socklen_t cmsg_len;		/* Length of data in cmsg_data plus length
>> +				   of cmsghdr structure.  */
>> +#else
>> +    socklen_t cmsg_len;
>> +    int __glibc_reserved1;
>> +#endif
>>      int cmsg_level;		/* Originating protocol.  */
>>      int cmsg_type;		/* Protocol specific type.  */
> ...
> 
>>  ssize_t
>>  __libc_sendmsg (int fd, const struct msghdr *msg, int flags)
>>  {
>> +  /* POSIX specifies that both msghdr::msg_iovlen and msghdr::msg_controllen
>> +     to be int and socklen_t respectively.  However Linux defines it as
>> +     both size_t.  So for 64-bit it requires some adjustments by copying to
>> +     temporary header and zeroing the pad fields.  */
>> +#if __WORDSIZE == 64
>> +  struct msghdr hdr;
>> +  if (msg != NULL)
>> +    {
>> +      hdr = *msg;
>> +      hdr.__glibc_reserved1 = 0;
>> +      hdr.__glibc_reserved2 = 0;
>> +      msg = &hdr;
>> +    }
>> +#endif
> 
> e.g. user supplied msg.msg_control might contain cmsghdr
> with __glibc_reserved1 != 0 since user code might not
> initialize the struct.
> 
> in musl this is fixed by copying the controls to a tmp
> buf on the stack (which has fixed size so it can fail)
> and fixing the paddings there.
> 

Yes I am aware and I noted this in my patch header:

 1. Current sendmsg fix does not handle larger msg_control neither
    pads the cmsghdr associated.  The problem with this approach
    is to accomplish a complete fix it will require to allocate
    a limited buffer, copying the incoming struct and zero pad.
    Although it tend to work it also add some limitation of total
    msg_control length.
    The general usage for such facily is passing file descriptors
    and permissions between processes over unix sockets so it might
    be factible to use a large stack allocated buffer (1024, 2048
    or large) and return ENOMEM for larger buffers.

I am planning to send a fix for this based on this patch.