netdev - Re: BUG: scheduling while atomic: ifconfig/0x00000002/4170

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: <1189171635.28781.134.camel@johannes.berg>
Date:	Fri, 07 Sep 2007 15:27:15 +0200
From:	Johannes Berg <johannes@...solutions.net>
To:	paulmck@...ux.vnet.ibm.com
Cc:	Herbert Xu <herbert@...dor.apana.org.au>, satyam@...radead.org,
	flo@...822.org, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, linux-wireless@...r.kernel.org,
	michal.k.k.piotrowski@...il.com,
	ipw3945-devel@...ts.sourceforge.net, yi.zhu@...el.com,
	flamingice@...rmilk.net
Subject: Re: BUG: scheduling while atomic: ifconfig/0x00000002/4170

On Thu, 2007-09-06 at 08:46 -0700, Paul E. McKenney wrote:

> Looks good to me from an RCU viewpoint.  I cannot claim familiarity with
> this code.  I therefore especially like the indications of where RTNL
> is held and not!!!

:)

> Some questions below based on a quick scan.  And a global question:
> should the comments about RTNL being held be replaced by ASSERT_RTNL()?

I don't like ASSERT_RTNL() much because it actually tries to lock it.
I'd be much happer if it was WARN_ON(!mutex_locked(&rtnl_mutex)) or
something equivalent.

In any case, I have an updated patch I'll be sending soon, and it
requires a new list walking primitive I'll also send.

> > -	write_lock_bh(&local->sub_if_lock);
> > +	/* we're under RTNL so all this is fine */
> >  	if (unlikely(local->reg_state == IEEE80211_DEV_UNREGISTERED)) {
> > -		write_unlock_bh(&local->sub_if_lock);
> >  		__ieee80211_if_del(local, sdata);
> >  		return -ENODEV;
> >  	}
> > -	list_add(&sdata->list, &local->sub_if_list);
> > +	list_add_tail_rcu(&sdata->list, &local->interfaces);
> 
> The _rcu is required because this list isn't protected by RTNL?

Yes, not all walkers of the list are protected by the RTNL.

> > @@ -226,22 +225,22 @@ void ieee80211_if_reinit(struct net_devi
> >  		/* Remove all virtual interfaces that use this BSS
> >  		 * as their sdata->bss */
> >  		struct ieee80211_sub_if_data *tsdata, *n;
> > -		LIST_HEAD(tmp_list);
> > 
> > -		write_lock_bh(&local->sub_if_lock);
> 
> This code is also protected by RTNL?

Yes.

> >  	ASSERT_RTNL();
> 
> I -like- this!!!  ;-)

:)

johannes

Download attachment "signature.asc" of type "application/pgp-signature" (191 bytes)