netdev - Re: [PATCH net-next 1/2] rtnetlink: move rtnl_lock handling out of af

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 for Android: free password hash cracker in your pocket

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

Date: Thu, 6 Jun 2024 17:18:15 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <kuba@...nel.org>
CC: <davem@...emloft.net>, <dsahern@...il.com>, <edumazet@...gle.com>,
	<jiri@...nulli.us>, <kuniyu@...zon.com>, <netdev@...r.kernel.org>,
	<pabeni@...hat.com>
Subject: Re: [PATCH net-next 1/2] rtnetlink: move rtnl_lock handling out of af_netlink

From: Jakub Kicinski <kuba@...nel.org>
Date: Thu, 6 Jun 2024 17:04:53 -0700
> On Thu, 6 Jun 2024 16:33:03 -0700 Kuniyuki Iwashima wrote:
> > > +	if (needs_lock)
> > > +		rtnl_lock();
> > >  	err = dumpit(skb, cb);
> > > +	if (needs_lock)
> > > +		rtnl_unlock();  
> > 
> > This calls netdev_run_todo() now, is this change intended ?
> 
> Nice catch / careful thinking, indeed we're moving from pure unlock to
> run_todo. I don't really recall if I thought of this when writing the
> change (it was few days back). My guess is that the fact we weren't
> calling full rtnl_unlock() was unintentional / out of laziness in the
> first place. It didn't matter since dumps are unlikely to changes /
> unregister / free things.

This makes sense.  Probably due to cb_mutex interface constraint.


> But still, someone may get caught off guard
> as some point that we're holding rtnl but won't go via the usual unlock
> path.
> 
> Would you like me to add a note to the commit message?

That would be nice, but I'm fine with this version :)

Reviewed-by: Kuniyuki Iwashima <kuniyu@...zon.com>

Thanks!