netdev - [PATCH (take 2)] vlan: lockdep class for ppp _xmit_lock Re: ppp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20070509131337.GA3177@ff.dom.local>
Date:	Wed, 9 May 2007 15:13:38 +0200
From:	Jarek Poplawski <jarkao2@...pl>
To:	David Miller <davem@...emloft.net>
Cc:	jura@...ams.com, akpm@...ux-foundation.org, netdev@...r.kernel.org,
	paulus@...ba.org, greearb@...delatech.com, mostrows@...akeasy.net
Subject: [PATCH (take 2)] vlan: lockdep class for ppp _xmit_lock Re: ppp_generic: fix lockdep warning

On Wed, May 09, 2007 at 02:32:24AM -0700, David Miller wrote:
> From: Jarek Poplawski <jarkao2@...pl>
> Date: Wed, 9 May 2007 11:35:37 +0200
> 
> > After rethinking there is the 3-rd way (as usual):
> > 
> > c) vlan should use different lockdep lock subclasses or
> > classes for different types of devices, used at the same
> > time.
> 
> Perhaps we should just bite the bullet and use a seperate
> unique lock class for ->_xmit_lock of each device type.
> 
> We are going to find more cases like this one, for sure.
> 

Theoretically subclasses are intended for this, and there
are 6 free places still. Practically lockdep treats subclasses
differently sometimes (e.g., now, it seems, it cannot see a real
recursion error inside a subclass). So, it's mainly a problem
of some static memory wasting or saving (unless it should be
subclassed yet - then no question).

Here is "your way" alternative, preferred version - I hope Yuriy
isn't bored with this testing yet!

Of course, I see nothing against you or somebody else doing
or re-doing this all as needed.

Regards,
Jarek P.
---> (take 2)

This patch's aim is to let lockdep see ppp devs as different
from others (default), and it's OK to take: _xmit_lock of vlan
and _xmit_lock of ppp with reverse order provided vlan _xmit_locks
are bound to different devs (ppp and e.g. eth).

> =======================================================
> [ INFO: possible circular locking dependency detected ]
> 2.6.21-rc6-mm1 #4
> -------------------------------------------------------
> pppd/14305 is trying to acquire lock:
>  (&vlan_netdev_xmit_lock_key){-...}, at: [<ffffffff8022f90b>]
> dev_queue_xmit+0x26b/0x300
>
> but task is already holding lock:
>  (&pch->downl#2){-+..}, at: [<ffffffff80388d3c>] ppp_push+0x5f/0xa7
>
> which lock already depends on the new lock.

The final idea to use separate classes by David Miller.

Reported & tested by: "Yuriy N. Shkandybin" <jura@...ams.com>
Cc: Ben Greear <greearb@...delatech.com>
Cc: Paul Mackerras <paulus@...ba.org>
Cc: Michal Ostrowski <mostrows@...akeasy.net>
Signed-off-by: Jarek Poplawski <jarkao2@...pl>

---

diff -Nurp 2.6.21-git7-/net/8021q/vlan.c 2.6.21/net/8021q/vlan.c
--- 2.6.21-git7-/net/8021q/vlan.c	2007-05-01 12:43:39.000000000 +0200
+++ 2.6.21/net/8021q/vlan.c	2007-05-09 14:46:12.000000000 +0200
@@ -376,6 +376,8 @@ static void vlan_transfer_operstate(cons
  */
 static struct lock_class_key vlan_netdev_xmit_lock_key;
 
+/* pppoe uses two different kinds of _xmit_lock for ppp & eth */
+static struct lock_class_key vlan_ppp_xmit_lock_key;
 
 /*  Attach a VLAN device to a mac address (ie Ethernet Card).
  *  Returns the device that was created, or NULL if there was
@@ -535,7 +537,12 @@ static struct net_device *register_vlan_
 	if (register_netdevice(new_dev))
 		goto out_free_newdev;
 
-	lockdep_set_class(&new_dev->_xmit_lock, &vlan_netdev_xmit_lock_key);
+	if (unlikely(real_dev->type == ARPHRD_PPP))
+		lockdep_set_class(&new_dev->_xmit_lock,
+				  &vlan_ppp_xmit_lock_key);
+	else
+		lockdep_set_class(&new_dev->_xmit_lock,
+				  &vlan_netdev_xmit_lock_key);
 
 	new_dev->iflink = real_dev->ifindex;
 	vlan_transfer_operstate(real_dev, new_dev);
-
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