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]
Date:	Wed, 15 Apr 2009 11:11:53 -0700
From:	Jay Vosburgh <fubar@...ibm.com>
To:	paul@...-scientist.net
cc:	netdev@...r.kernel.org
Subject: Re: 2.6.27.18: bnx2/tg3: BUG: "scheduling while atomic" trying to ifenslave a second interface to my bond

Paul Smith <paul@...-scientist.net> wrote:

>On Tue, 2009-04-14 at 18:12 -0700, Jay Vosburgh wrote:
>> 	I think I know what's going on.  I believe this patch will
>> resolve things, but I won't be able to test it until tomorrow.  If you
>> want to test this, great; if you want to wait, that's fine too.
>
>Hi Jay; as I mentioned last night this patch is working fine for me so
>far.

	Thanks for the test report.

>However, looking at the rest of this function it seems to me that there
>are other locking issues, at least based on the documentation in the
>header file:
>
> * Here are the locking policies for the two bonding locks:
> *
> * 1) Get bond->lock when reading/writing slave list.
> * 2) Get bond->curr_slave_lock when reading/writing bond->curr_active_slave.
> *    (It is unnecessary when the write-lock is put with bond->lock.)
> * 3) When we lock with bond->curr_slave_lock, we must lock with bond->lock
> *    beforehand.
>
>For example, don't you need to hold bond->curr_slave_lock at least
>around the "if (!bond->curr_active_slave)"?  What about around the
>"bond_for_each_slave" loop?
>
>Many of the other functions, later, also seem to work with
>bond->curr_active_slave and they don't take this lock.
>
>Unless I'm missing something, I think there are still more problems in
>the locking in bond_alb_set_mac_address().

	The various MAC manipulating functions are either called under
RTNL (as bond_alb_set_mac_address is) or take pains to acquire RTNL
before doing anything with the MAC.  Also, the slave list and
curr_active_slave are mutexed by RTNL, so those inspections should be
safe.

	I'm reasonably sure that the curr_slave_lock is superfluous
(which wasn't the case when it was originally introduced), but I haven't
had a chance to validate this.  The locking has changed from what's
documented in the header file; RTNL wasn't used for this when that was
written.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com
--
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ