Re: buggy check in netlink_mmap_sendmsg()

[Al Viro <viro@ZenIV.linux.org.uk>]

On Fri, Jul 19, 2013 at 05:38:46PM +0200, Patrick McHardy wrote:
> On Thu, Jul 18, 2013 at 01:13:58PM +0200, Patrick McHardy wrote:
> > 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.

Sorry, no.  Thread A: calls sendmsg(), gets past the check, loses CPU1 on
e.g. preempt.  Thread B on CPU2 calls mmap(2), returns to userland (having
dropped the mutex, of course) and proceeds to play with the mapped area.
Thread A regains CPU and moves on past that if (....), excl still being true.
See the problem?