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  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Sun, 17 Jan 2021 11:12:42 +0100
From:   Uwe Kleine-König <>
To:     Dany Madden <>, Lijun Pan <>,
        Sukadev Bhattiprolu <>,
        Michael Ellerman <>,
        Juliet Kim <>
Cc:     Benjamin Herrenschmidt <>,
        Paul Mackerras <>,
        "David S. Miller" <>,
        Jakub Kicinski <>,,,
        Greg Kroah-Hartman <>,
Subject: ibmvnic: Race condition in remove callback


while working on some cleanup I stumbled over a problem in the ibmvnic's
remove callback. Since commit

        7d7195a026ba ("ibmvnic: Do not process device remove during device reset")

there is the following code in the remove callback:

        static int ibmvnic_remove(struct vio_dev *dev)
                spin_lock_irqsave(&adapter->state_lock, flags);
                if (test_bit(0, &adapter->resetting)) {
                        spin_unlock_irqrestore(&adapter->state_lock, flags);
                        return -EBUSY;

                adapter->state = VNIC_REMOVING;
                spin_unlock_irqrestore(&adapter->state_lock, flags);


Unfortunately returning -EBUSY doesn't work as intended. That's because
the return value of this function is ignored[1] and the device is
considered unbound by the device core (shortly) after ibmvnic_remove()

While looking into fixing that I noticed a worse problem:

If ibmvnic_reset() (e.g. called by the tx_timeout callback) calls
schedule_work(&adapter->ibmvnic_reset); just after the work queue is
flushed above the problem that 7d7195a026ba intends to fix will trigger
resulting in a use-after-free.

Also ibmvnic_reset() checks for adapter->state without holding the lock
which might be racy, too.

Best regards

[1] vio_bus_remove (in arch/powerpc/platforms/pseries/vio.c) records the
    return value and passes it on. But the driver core doesn't care for
    the return value (see __device_release_driver() in drivers/base/dd.c
    calling dev->bus->remove()).

Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | |

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

Powered by blists - more mailing lists