[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130718111358.GA27488@macbook.localnet>
Date: Thu, 18 Jul 2013 13:22:34 +0200
From: Patrick McHardy <kaber@...sh.net>
To: Al Viro <viro@...IV.linux.org.uk>
Cc: netdev@...r.kernel.org
Subject: Re: buggy check in netlink_mmap_sendmsg()
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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists