netdev - Re: Strangeness in ehea network driver's shutdown

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: <20221207082301.vfkh2zoty54rhhsv@pengutronix.de>
Date:   Wed, 7 Dec 2022 09:23:01 +0100
From:   Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To:     Thadeu Lima de Souza Cascardo <cascardo@...onical.com>
Cc:     "Guilherme G. Piccoli" <kernel@...ccoli.net>,
        "Rafael J. Wysocki" <rafael@...nel.org>, netdev@...r.kernel.org,
        Douglas Miller <dougmill@...ux.ibm.com>, gpiccoli@...lia.com,
        kernel@...gutronix.de,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jakub Kicinski <kuba@...nel.org>
Subject: Re: Strangeness in ehea network driver's shutdown

On Tue, Dec 06, 2022 at 02:02:09PM -0300, Thadeu Lima de Souza Cascardo wrote:
> On Tue, Dec 06, 2022 at 01:49:01PM -0300, Guilherme G. Piccoli wrote:
> > On Mon, Oct 3, 2022 at 1:36 PM Jakub Kicinski <kuba@...nel.org> wrote:
> > >
> > > On Sat, 1 Oct 2022 16:31:31 +0200 Uwe Kleine-König wrote:
> > > > Hello,
> > > >
> > > > while doing some cleanup I stumbled over a problem in the ehea network
> > > > driver.
> > > >
> > > > In the driver's probe function (ehea_probe_adapter() via
> > > > ehea_register_memory_hooks()) a reboot notifier is registered. When this
> > > > notifier is triggered (ehea_reboot_notifier()) it unregisters the
> > > > driver. I'm unsure what is the order of the actions triggered by that.
> > > > Maybe the driver is unregistered twice if there are two bound devices?
> 
> I see how you would think it might be called for every bound device. That's
> because ehea_register_memory_hooks is called by ehea_probe_adapter. However,
> there is this test here that leads it the reboot_notifier to be registered only
> once:
> 
> [...]
> static int ehea_register_memory_hooks(void)
> {
> 	int ret = 0;
> 
> 	if (atomic_inc_return(&ehea_memory_hooks_registered) > 1)
> 	^^^^^^^^^^^^^^^^^^^^^^
> 		return 0;
> [...]

Ah, I see.

> > > > Or the reboot notifier is called under a lock and unregistering the
> > > > driver (and so the devices) tries to unregister the notifier that is
> > > > currently locked and so results in a deadlock? Maybe Greg or Rafael can
> > > > tell about the details here?
> > > >
> > > > Whatever the effect is, it's strange. It makes me wonder why it's
> > > > necessary to free all the resources of the driver on reboot?! I don't
> 
> As for why:
> 
> commit 2a6f4e4983918b18fe5d3fb364afe33db7139870
> Author: Jan-Bernd Themann <ossthema@...ibm.com>
> Date:   Fri Oct 26 14:37:28 2007 +0200
> 
>     ehea: add kexec support
>     
>     eHEA resources that are allocated via H_CALLs have a unique identifier each.
>     These identifiers are necessary to free the resources. A reboot notifier
>     is used to free all eHEA resources before the indentifiers get lost, i.e
>     before kexec starts a new kernel.
>     
>     Signed-off-by: Jan-Bernd Themann <themann@...ibm.com>
>     Signed-off-by: Jeff Garzik <jeff@...zik.org>

I don't understand that, but that's fine for me.

As you're happy with the state as is, I consider the Case closed. Thanks
for looking into my bug report.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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