Re: buggy check in netlink_mmap_sendmsg()

[Patrick McHardy <kaber@trash.net>]

On Sun, Jul 14, 2013 at 10:36:19AM +0100, Al Viro wrote:
> This
>         /* Netlink messages are validated by the receiver before processing.
>          * In order to avoid userspace changing the contents of the message
>          * after validation, the socket and the ring may only be used by a
>          * single process, otherwise we fall back to copying.
>          */
>         if (atomic_long_read(&sk->sk_socket->file->f_count) > 2 ||  
>             atomic_read(&nlk->mapped) > 1)
>                 excl = false;
> looks very odd.  For one thing, descriptor table may be shared, with
> one thread calling sendmsg() (which gives f_count equal to 2), while
> another calls mmap() just as the first one gets past that check.

Another thread calling mmap() should be fine since validation, processing
and mmap() all happen under the pg_vec_lock mutex.

> Moreover, we might very well have the damn thing mmapped, then clone(2)
> creating another thread that shares address space, but not the descriptor
> table.  Child closes the socket descriptor it got, then parent does
> sendmsg(2) (f_count == 2, again, since this time descriptor table isn't
> shared and sendmsg(2) doesn't grab a reference and we have 1 from descriptor
> table and 1 from mapping).  Again, the child has it mapped and can play
> with it as it wishes...

This is unfortunately not my area of expertise. Let me look into how we
can prevent this.