On Tue, Jan 27, 2009 at 09:52:49AM +0100, Peter Zijlstra wrote:

On Tue, 2009-01-27 at 08:45 +0000, Jarek Poplawski wrote:

quoted

On Mon, Jan 26, 2009 at 10:30:30PM +0100, Martin MOKREJŠ wrote:

quoted

The patch really did not help:
http://bugzilla.kernel.org/show_bug.cgi?id=12515#c5
Martin

Actually, there is a little change: the warning triggerd in another
place (sock_setsockopt() -> sk_attach_filter()). So we could go deeper
with these changes, but I'm not sure this is the right way to fix.

It looks like the scenario is very old, but probably wasn't reported
(maybe there is some lockdep improvement):

Yes, they likely are very old, and yes we added a lockdep annotation to
copy_to/from_user() to catch these.

quoted

A) sys_mmap2() -> mm->mmap_sem -> packet_mmap() -> sk_lock
B) sock_setsockopt() -> sk_lock -> copy_from_user() -> mm->mmap_sem

packet_mmap() (net/packet/af_packet.c) seems to be the only place in
net to implement mmap method, and using this lock order btw. On the
other hand copy_from_user() could be more popular under sk_lock, and
I'm not sure these changes are necessary.

Since I don't know enough neither sock/packet nor sys_mmap, I guess
some advice would be precious. It looks like Peter Zijlstra solved
similar problems in nfs, so I CC him.

The NFS/sunrpc case was special in that it did copy_to/from_kernel, that
is, it never actually touched user memory -- we taught the might_fault()
annotation about that.

Can't you simply do the copy_from_user() before you take the sk_lock?

Since it's really needed, and Vegard started doing it like this, I
guess he will try to add the missing pieces.

Thanks again,
Jarek P.