Thread (9 messages) 9 messages, 5 authors, 2009-01-30

Re: [Bugme-new] [Bug 12515] New: possible circular locking #0: (sk_lock-AF_PACKET){--..}, at: [<c1279838>] sock_setsockopt+0x12b/0x4a4

From: Martin MOKREJŠ <hidden>
Date: 2009-01-27 21:53:52

There has been some discussion under subject
"Re: [PATCH] net: fix setsockopt() locking errors"
and I hope it is archived somewhere via netdev@vger.kernel.org .
If not, Jarek Poplawski and Peter Zijlstra have some clues
what to do and I am waiting for Vegard Nossum to give me
another patch to test.


Here is some part of the discussion ,hopefully the most important
part:

On Tue, Jan 27, 2009 at 09:52:49AM +0100, Peter Zijlstra wrote:
quoted
On Tue, 2009-01-27 at 08:45 +0000, Jarek Poplawski wrote:
quoted
quoted
On Mon, Jan 26, 2009 at 10:30:30PM +0100, Martin MOKREJŠ wrote:
quoted
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
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.





Andrew Morton wrote:
(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Tue, 20 Jan 2009 10:16:38 -0800 (PST)
bugme-daemon@bugzilla.kernel.org wrote:
quoted
http://bugzilla.kernel.org/show_bug.cgi?id=12515

           Summary: possible circular locking #0:  (sk_lock-AF_PACKET){--
                    ..}, at: [<c1279838>] sock_setsockopt+0x12b/0x4a4
           Product: Networking
           Version: 2.5
     KernelVersion: 2.6.29-rc1-git4
          Platform: All
        OS/Version: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: IPV4
        AssignedTo: shemminger@linux-foundation.org
        ReportedBy: mmokrejs@ribosome.natur.cuni.cz


Latest working kernel version:
Earliest failing kernel version:
Distribution: Gentoo Linux
Hardware Environment: ASUS L3C/S laptop
Software Environment:
Problem Description:

Steps to reproduce: I have been reinstalling some apps while started tcpdump.
It immediately hit the bug.
More info at the link.  Vegard did some analysis, had a shot at fixing
it, but it seems that he missed.
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help