netdev - Re: [RFC PATCH net-next 1/4] net-sysfs: remove rtnl

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: <173626740387.3685.11436966751545966054@kwain>
Date: Tue, 07 Jan 2025 17:30:03 +0100
From: Antoine Tenart <atenart@...nel.org>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, pabeni@...hat.com, edumazet@...gle.com, netdev@...r.kernel.org, gregkh@...uxfoundation.org, mhocko@...e.com, stephen@...workplumber.org
Subject: Re: [RFC PATCH net-next 1/4] net-sysfs: remove rtnl_trylock from device attributes

Hi Jakub,

Quoting Jakub Kicinski (2025-01-02 23:36:47)
> On Wed, 18 Oct 2023 17:47:43 +0200 Antoine Tenart wrote:
> > We have an ABBA deadlock between net device unregistration and sysfs
> > files being accessed[1][2]. To prevent this from happening all paths
> > taking the rtnl lock after the sysfs one (actually kn->active refcount)
> > use rtnl_trylock and return early (using restart_syscall)[3] which can
> > make syscalls to spin for a long time when there is contention on the
> > rtnl lock[4].
> 
> I was looking at the sysfs locking, and ended up going down a very
> similar path. Luckily lore search for sysfs_break_active_protection()
> surfaced this thread so I can save myself some duplicated work :)

Seeing that thread in my inbox again is a nice surprise :-)

Did you encounter any specific issue that made you look at the sysfs
locking?

> Is there any particular reason why you haven't pursued this solution
> further? I think it should work.

I felt there wasn't much interest and feedback at the time and we had
things in place to ease the initial issue we were working on (~ slow
boot time w/ lots of netns and containers). With that and given the
change was a bit tricky I didn't wanted to be the only one pushing for
this.

But I still think this could be beneficial for various use cases so if
you're interested I'll be happy to revive it. I'll have to refresh my
mind and run some tests again first. (Any additional testing will be
appreciated too).

> My version, FWIW:
> https://github.com/kuba-moo/linux/commit/2724bb7275496a254b001fe06fe20ccc5addc9d2

I might take a few of your changes in there, eg. I see you used an
interruptible lock. With this and the few minors comments this RFC got I
can prepare a new series.

Thanks!
Antoine