netdev - Re: [PATCH net] r8169: fix LED-related deadlock on module removal

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] [day] [month] [year] [list]

Message-ID: <2024041830-entertain-platonic-b741@gregkh>
Date: Thu, 18 Apr 2024 16:44:22 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Heiner Kallweit <hkallweit1@...il.com>
Cc: Lukas Wunner <lukas@...ner.de>, Jakub Kicinski <kuba@...nel.org>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net] r8169: fix LED-related deadlock on module removal

On Thu, Apr 18, 2024 at 04:33:37PM +0200, Heiner Kallweit wrote:
> On Thu, Apr 18, 2024 at 11:55 AM Greg KH <gregkh@...uxfoundation.org> wrote:
> >
> > On Thu, Apr 18, 2024 at 12:33:00AM +0200, Lukas Wunner wrote:
> > > On Wed, Apr 17, 2024 at 09:43:27AM +0200, Greg KH wrote:
> > > > On Wed, Apr 17, 2024 at 09:16:04AM +0200, Heiner Kallweit wrote:
> > > > > On 17.04.2024 09:04, Greg KH wrote:
> > > > > > On Wed, Apr 17, 2024 at 08:02:31AM +0200, Heiner Kallweit wrote:
> > > > > >> On 17.04.2024 04:34, Jakub Kicinski wrote:
> > > > > >>> On Mon, 15 Apr 2024 13:57:17 +0200 Heiner Kallweit wrote:
> > > > > >>>> Binding devm_led_classdev_register() to the netdev is problematic
> > > > > >>>> because on module removal we get a RTNL-related deadlock. Fix this
> > > > > >>>> by avoiding the device-managed LED functions.
> > > > > >>>>
> > > > > >>>> Note: We can safely call led_classdev_unregister() for a LED even
> > > > > >>>> if registering it failed, because led_classdev_unregister() detects
> > > > > >>>> this and is a no-op in this case.
> > > > > >>>>
> > > > > >>>> Fixes: 18764b883e15 ("r8169: add support for LED's on RTL8168/RTL8101")
> > > > > >>>> Cc: <stable@...r.kernel.org> # 6.8.x
> > > > > >>>> Reported-by: Lukas Wunner <lukas@...ner.de>
> > > > > >>>> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
> > > > > >>
> > > > > >> This is a version of the fix modified to apply on 6.8.
> > > > > >
> > > > > > That was not obvious at all :(
> > > > > >
> > > > > Stating "Cc: <stable@...r.kernel.org> # 6.8.x" isn't sufficient?
> > > >
> > > > Without showing what commit id this is in Linus's tree, no.
> > >
> > > The upstream commit id *is* called out in the patch, but it's buried
> > > below the three dashes:
> > >
> > >     The original change was introduced with 6.8, 6.9 added support for
> > >     LEDs on RTL8125. Therefore the first version of the fix applied on
> > >     6.9-rc only. This is the modified version for 6.8.
> > >     Upstream commit: 19fa4f2a85d7
> > >                      ^^^^^^^^^^^^
> > >
> > > The proper way to do this is to prominently add ...
> > >
> > >     commit 19fa4f2a85d777a8052e869c1b892a2f7556569d upstream.
> > >
> > > ... or ...
> > >
> > >     [ Upstream commit 19fa4f2a85d777a8052e869c1b892a2f7556569d ]
> > >
> > > ... as the first line of the commit message, as per
> > > Documentation/process/stable-kernel-rules.rst
> > >
> >
> > Yes, Heiner, please resubmit this, AND submit the fix-for-this-fix as
> > well, so that if we take this patch, it is not broken.
> >
> OK. The fix-for-the-fix was included already.

Included where?  In this change?  Please do not do that.

> It's trivial and IMO submitting it
> separately would just create overhead.

Not submitting it would cause problems when people look and see that the
"fix" is not also applied and then you would get automated emails
complaining about it.

Mirror what is in Linus's tree whenever possible please, it's simpler
and saves EVERYONE extra work.

thanks,

greg k-h