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: Windows password security audit tool. GUI, reports in PDF.

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

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

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. 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?