netdev - Re: [PATCH net] ibmvnic: device remove has higher precedence over reset

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: <20210121223810.GA374395@us.ibm.com>
Date:   Thu, 21 Jan 2021 14:38:10 -0800
From:   Sukadev Bhattiprolu <sukadev@...ux.ibm.com>
To:     Lijun Pan <lijunp213@...il.com>
Cc:     Dany Madden <drt@...ux.ibm.com>, Lijun Pan <ljp@...ux.ibm.com>,
        netdev@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        mpe@...erman.id.au, julietk@...ux.vnet.ibm.com,
        benh@...nel.crashing.org, paulus@...ba.org, davem@...emloft.net,
        Jakub Kicinski <kuba@...nel.org>, gregkh@...uxfoundation.org,
        kernel@...gutronix.de,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Subject: Re: [PATCH net] ibmvnic: device remove has higher precedence over
 reset

Lijun Pan [lijunp213@...il.com] wrote:
> > > diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
> > > b/drivers/net/ethernet/ibm/ibmvnic.c
> > > index aed985e08e8a..11f28fd03057 100644
> > > --- a/drivers/net/ethernet/ibm/ibmvnic.c
> > > +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> > > @@ -2235,8 +2235,7 @@ static void __ibmvnic_reset(struct work_struct
> > > *work)
> > >       while (rwi) {
> > >               spin_lock_irqsave(&adapter->state_lock, flags);
> > >
> > > -             if (adapter->state == VNIC_REMOVING ||
> > > -                 adapter->state == VNIC_REMOVED) {
> > > +             if (adapter->state == VNIC_REMOVED) {

If the adapter is in REMOVING state, there is no point going
through the reset process. We could just bail out here. We
should also drain any other resets in the queue (something
my other patch set was addressing).

Sukadev

> >
> > If we do get here, we would crash because ibmvnic_remove() happened. It
> > frees the adapter struct already.
> 
> Not exactly. viodev is gone; netdev is gone; ibmvnic_adapter is still there.
> 
> Lijun