Fix a potential deadlock if disconnect races with open.
Since commit d4ead16f50f9 ("USB: prevent char device open/deregister
race") core holds an rw-semaphore while open is called and when
releasing the minor number during deregistration. This can lead to an
ABBA deadlock if a driver takes a lock in open which it also holds
during deregistration.
This effectively reverts commit 78663ecc344b ("USB: disconnect open race
in legousbtower") which needlessly introduced this issue after a generic
fix for this race had been added to core by commit d4ead16f50f9 ("USB:
prevent char device open/deregister race").
Fixes: 78663ecc344b ("USB: disconnect open race in legousbtower")
Cc: stable <stable(a)vger.kernel.org> # 2.6.24
Reported-by: syzbot+f9549f5ee8a5416f0b95(a)syzkaller.appspotmail.com
Tested-by: syzbot+f9549f5ee8a5416f0b95(a)syzkaller.appspotmail.com
Signed-off-by: Johan Hovold <johan(a)kernel.org>
---
drivers/usb/misc/legousbtower.c | 19 ++-----------------
1 file changed, 2 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
index 1db07d4dc738..773e4188f336 100644
--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -179,7 +179,6 @@ static const struct usb_device_id tower_table[] = {
};
MODULE_DEVICE_TABLE (usb, tower_table);
-static DEFINE_MUTEX(open_disc_mutex);
#define LEGO_USB_TOWER_MINOR_BASE 160
@@ -332,18 +331,14 @@ static int tower_open (struct inode *inode, struct file *file)
goto exit;
}
- mutex_lock(&open_disc_mutex);
dev = usb_get_intfdata(interface);
-
if (!dev) {
- mutex_unlock(&open_disc_mutex);
retval = -ENODEV;
goto exit;
}
/* lock this device */
if (mutex_lock_interruptible(&dev->lock)) {
- mutex_unlock(&open_disc_mutex);
retval = -ERESTARTSYS;
goto exit;
}
@@ -351,12 +346,10 @@ static int tower_open (struct inode *inode, struct file *file)
/* allow opening only once */
if (dev->open_count) {
- mutex_unlock(&open_disc_mutex);
retval = -EBUSY;
goto unlock_exit;
}
dev->open_count = 1;
- mutex_unlock(&open_disc_mutex);
/* reset the tower */
result = usb_control_msg (dev->udev,
@@ -423,10 +416,9 @@ static int tower_release (struct inode *inode, struct file *file)
if (dev == NULL) {
retval = -ENODEV;
- goto exit_nolock;
+ goto exit;
}
- mutex_lock(&open_disc_mutex);
if (mutex_lock_interruptible(&dev->lock)) {
retval = -ERESTARTSYS;
goto exit;
@@ -456,10 +448,7 @@ static int tower_release (struct inode *inode, struct file *file)
unlock_exit:
mutex_unlock(&dev->lock);
-
exit:
- mutex_unlock(&open_disc_mutex);
-exit_nolock:
return retval;
}
@@ -912,7 +901,6 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
if (retval) {
/* something prevented us from registering this driver */
dev_err(idev, "Not able to get a minor for this device.\n");
- usb_set_intfdata (interface, NULL);
goto error;
}
dev->minor = interface->minor;
@@ -944,16 +932,13 @@ static void tower_disconnect (struct usb_interface *interface)
int minor;
dev = usb_get_intfdata (interface);
- mutex_lock(&open_disc_mutex);
- usb_set_intfdata (interface, NULL);
minor = dev->minor;
- /* give back our minor */
+ /* give back our minor and prevent further open() */
usb_deregister_dev (interface, &tower_class);
mutex_lock(&dev->lock);
- mutex_unlock(&open_disc_mutex);
/* if the device is not opened, then we clean up right now */
if (!dev->open_count) {
--
2.23.0
Make sure to check for short transfers when retrieving the version
information at probe to avoid leaking uninitialised slab data when
logging it.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable(a)vger.kernel.org>
Signed-off-by: Johan Hovold <johan(a)kernel.org>
---
drivers/usb/misc/legousbtower.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
index 006cf13b2199..1db07d4dc738 100644
--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -891,8 +891,10 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
get_version_reply,
sizeof(*get_version_reply),
1000);
- if (result < 0) {
- dev_err(idev, "LEGO USB Tower get version control request failed\n");
+ if (result < sizeof(*get_version_reply)) {
+ if (result >= 0)
+ result = -EIO;
+ dev_err(idev, "get version request failed: %d\n", result);
retval = result;
goto error;
}
--
2.23.0
The driver is using its struct usb_device pointer as an inverted
disconnected flag, but was setting it to NULL before making sure all
completion handlers had run. This could lead to a NULL-pointer
dereference in a number of dev_dbg and dev_err statements in the
completion handlers which relies on said pointer.
Fix this by unconditionally stopping all I/O and preventing
resubmissions by poisoning the interrupt URBs at disconnect and using a
dedicated disconnected flag.
This also makes sure that all I/O has completed by the time the
disconnect callback returns.
Fixes: 9d974b2a06e3 ("USB: legousbtower.c: remove err() usage")
Fixes: fef526cae700 ("USB: legousbtower: remove custom debug macro")
Fixes: 4dae99638097 ("USB: legotower: remove custom debug macro and module parameter")
Cc: stable <stable(a)vger.kernel.org> # 3.5
Signed-off-by: Johan Hovold <johan(a)kernel.org>
---
drivers/usb/misc/legousbtower.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
index 773e4188f336..4fa999882635 100644
--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -190,6 +190,7 @@ struct lego_usb_tower {
unsigned char minor; /* the starting minor number for this device */
int open_count; /* number of times this port has been opened */
+ unsigned long disconnected:1;
char* read_buffer;
size_t read_buffer_length; /* this much came in */
@@ -289,8 +290,6 @@ static inline void lego_usb_tower_debug_data(struct device *dev,
*/
static inline void tower_delete (struct lego_usb_tower *dev)
{
- tower_abort_transfers (dev);
-
/* free data structures */
usb_free_urb(dev->interrupt_in_urb);
usb_free_urb(dev->interrupt_out_urb);
@@ -430,7 +429,8 @@ static int tower_release (struct inode *inode, struct file *file)
retval = -ENODEV;
goto unlock_exit;
}
- if (dev->udev == NULL) {
+
+ if (dev->disconnected) {
/* the device was unplugged before the file was released */
/* unlock here as tower_delete frees dev */
@@ -466,10 +466,9 @@ static void tower_abort_transfers (struct lego_usb_tower *dev)
if (dev->interrupt_in_running) {
dev->interrupt_in_running = 0;
mb();
- if (dev->udev)
- usb_kill_urb (dev->interrupt_in_urb);
+ usb_kill_urb(dev->interrupt_in_urb);
}
- if (dev->interrupt_out_busy && dev->udev)
+ if (dev->interrupt_out_busy)
usb_kill_urb(dev->interrupt_out_urb);
}
@@ -505,7 +504,7 @@ static __poll_t tower_poll (struct file *file, poll_table *wait)
dev = file->private_data;
- if (!dev->udev)
+ if (dev->disconnected)
return EPOLLERR | EPOLLHUP;
poll_wait(file, &dev->read_wait, wait);
@@ -552,7 +551,7 @@ static ssize_t tower_read (struct file *file, char __user *buffer, size_t count,
}
/* verify that the device wasn't unplugged */
- if (dev->udev == NULL) {
+ if (dev->disconnected) {
retval = -ENODEV;
pr_err("No device or device unplugged %d\n", retval);
goto unlock_exit;
@@ -638,7 +637,7 @@ static ssize_t tower_write (struct file *file, const char __user *buffer, size_t
}
/* verify that the device wasn't unplugged */
- if (dev->udev == NULL) {
+ if (dev->disconnected) {
retval = -ENODEV;
pr_err("No device or device unplugged %d\n", retval);
goto unlock_exit;
@@ -748,7 +747,7 @@ static void tower_interrupt_in_callback (struct urb *urb)
resubmit:
/* resubmit if we're still running */
- if (dev->interrupt_in_running && dev->udev) {
+ if (dev->interrupt_in_running) {
retval = usb_submit_urb (dev->interrupt_in_urb, GFP_ATOMIC);
if (retval)
dev_err(&dev->udev->dev,
@@ -813,6 +812,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
dev->udev = udev;
dev->open_count = 0;
+ dev->disconnected = 0;
dev->read_buffer = NULL;
dev->read_buffer_length = 0;
@@ -938,6 +938,10 @@ static void tower_disconnect (struct usb_interface *interface)
/* give back our minor and prevent further open() */
usb_deregister_dev (interface, &tower_class);
+ /* stop I/O */
+ usb_poison_urb(dev->interrupt_in_urb);
+ usb_poison_urb(dev->interrupt_out_urb);
+
mutex_lock(&dev->lock);
/* if the device is not opened, then we clean up right now */
@@ -945,7 +949,7 @@ static void tower_disconnect (struct usb_interface *interface)
mutex_unlock(&dev->lock);
tower_delete (dev);
} else {
- dev->udev = NULL;
+ dev->disconnected = 1;
/* wake up pollers */
wake_up_interruptible_all(&dev->read_wait);
wake_up_interruptible_all(&dev->write_wait);
--
2.23.0
Fix a potential deadlock if disconnect races with open.
Since commit d4ead16f50f9 ("USB: prevent char device open/deregister
race") core holds an rw-semaphore while open is called and when
releasing the minor number during deregistration. This can lead to an
ABBA deadlock if a driver takes a lock in open which it also holds
during deregistration.
This effectively reverts commit 78663ecc344b ("USB: disconnect open race
in legousbtower") which needlessly introduced this issue after a generic
fix for this race had been added to core by commit d4ead16f50f9 ("USB:
prevent char device open/deregister race").
Fixes: 78663ecc344b ("USB: disconnect open race in legousbtower")
Cc: stable <stable(a)vger.kernel.org> # 2.6.24
Reported-by: syzbot+f9549f5ee8a5416f0b95(a)syzkaller.appspotmail.com
Tested-by: syzbot+f9549f5ee8a5416f0b95(a)syzkaller.appspotmail.com
Signed-off-by: Johan Hovold <johan(a)kernel.org>
---
drivers/usb/misc/legousbtower.c | 19 ++-----------------
1 file changed, 2 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
index 1db07d4dc738..773e4188f336 100644
--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -179,7 +179,6 @@ static const struct usb_device_id tower_table[] = {
};
MODULE_DEVICE_TABLE (usb, tower_table);
-static DEFINE_MUTEX(open_disc_mutex);
#define LEGO_USB_TOWER_MINOR_BASE 160
@@ -332,18 +331,14 @@ static int tower_open (struct inode *inode, struct file *file)
goto exit;
}
- mutex_lock(&open_disc_mutex);
dev = usb_get_intfdata(interface);
-
if (!dev) {
- mutex_unlock(&open_disc_mutex);
retval = -ENODEV;
goto exit;
}
/* lock this device */
if (mutex_lock_interruptible(&dev->lock)) {
- mutex_unlock(&open_disc_mutex);
retval = -ERESTARTSYS;
goto exit;
}
@@ -351,12 +346,10 @@ static int tower_open (struct inode *inode, struct file *file)
/* allow opening only once */
if (dev->open_count) {
- mutex_unlock(&open_disc_mutex);
retval = -EBUSY;
goto unlock_exit;
}
dev->open_count = 1;
- mutex_unlock(&open_disc_mutex);
/* reset the tower */
result = usb_control_msg (dev->udev,
@@ -423,10 +416,9 @@ static int tower_release (struct inode *inode, struct file *file)
if (dev == NULL) {
retval = -ENODEV;
- goto exit_nolock;
+ goto exit;
}
- mutex_lock(&open_disc_mutex);
if (mutex_lock_interruptible(&dev->lock)) {
retval = -ERESTARTSYS;
goto exit;
@@ -456,10 +448,7 @@ static int tower_release (struct inode *inode, struct file *file)
unlock_exit:
mutex_unlock(&dev->lock);
-
exit:
- mutex_unlock(&open_disc_mutex);
-exit_nolock:
return retval;
}
@@ -912,7 +901,6 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
if (retval) {
/* something prevented us from registering this driver */
dev_err(idev, "Not able to get a minor for this device.\n");
- usb_set_intfdata (interface, NULL);
goto error;
}
dev->minor = interface->minor;
@@ -944,16 +932,13 @@ static void tower_disconnect (struct usb_interface *interface)
int minor;
dev = usb_get_intfdata (interface);
- mutex_lock(&open_disc_mutex);
- usb_set_intfdata (interface, NULL);
minor = dev->minor;
- /* give back our minor */
+ /* give back our minor and prevent further open() */
usb_deregister_dev (interface, &tower_class);
mutex_lock(&dev->lock);
- mutex_unlock(&open_disc_mutex);
/* if the device is not opened, then we clean up right now */
if (!dev->open_count) {
--
2.23.0
Make sure to check for short transfers when retrieving the version
information at probe to avoid leaking uninitialised slab data when
logging it.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable(a)vger.kernel.org>
Signed-off-by: Johan Hovold <johan(a)kernel.org>
---
drivers/usb/misc/legousbtower.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
index 006cf13b2199..1db07d4dc738 100644
--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -891,8 +891,10 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
get_version_reply,
sizeof(*get_version_reply),
1000);
- if (result < 0) {
- dev_err(idev, "LEGO USB Tower get version control request failed\n");
+ if (result < sizeof(*get_version_reply)) {
+ if (result >= 0)
+ result = -EIO;
+ dev_err(idev, "get version request failed: %d\n", result);
retval = result;
goto error;
}
--
2.23.0
This is the start of the stable review cycle for the 4.14.145 release.
There are 45 patches in this series, all will be posted as a response
to this one. If anyone has any issues with these being applied, please
let me know.
Responses should be made by Fri 20 Sep 2019 06:09:47 AM UTC.
Anything received after that time might be too late.
The whole patch series can be found in one patch at:
https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.14.145-r…
or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-4.14.y
and the diffstat can be found below.
thanks,
greg k-h
-------------
Pseudo-Shortlog of commits:
Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
Linux 4.14.145-rc1
Linus Torvalds <torvalds(a)linux-foundation.org>
x86/build: Add -Wnoaddress-of-packed-member to REALMODE_CFLAGS, to silence GCC9 build warning
Jean Delvare <jdelvare(a)suse.de>
nvmem: Use the same permissions for eeprom as for nvmem
Steffen Dirkwinkel <s.dirkwinkel(a)beckhoff.com>
platform/x86: pmc_atom: Add CB4063 Beckhoff Automation board to critclk_systems DMI table
Mario Limonciello <mario.limonciello(a)dell.com>
Revert "Bluetooth: btusb: driver to enable the usb-wakeup feature"
Nishka Dasgupta <nishkadg.linux(a)gmail.com>
drm/mediatek: mtk_drm_drv.c: Add of_node_put() before goto
Andrew F. Davis <afd(a)ti.com>
firmware: ti_sci: Always request response from firmware
Christophe Leroy <christophe.leroy(a)c-s.fr>
crypto: talitos - HMAC SNOOP NO AFEU mode requires SW icv checking.
Christophe Leroy <christophe.leroy(a)c-s.fr>
crypto: talitos - Do not modify req->cryptlen on decryption.
Christophe Leroy <christophe.leroy(a)c-s.fr>
crypto: talitos - fix ECB algs ivsize
Christophe Leroy <christophe.leroy(a)c-s.fr>
crypto: talitos - check data blocksize in ablkcipher.
Christophe Leroy <christophe.leroy(a)c-s.fr>
crypto: talitos - fix CTR alg blocksize
Christophe Leroy <christophe.leroy(a)c-s.fr>
crypto: talitos - check AES key size
Muchun Song <smuchun(a)gmail.com>
driver core: Fix use-after-free and double free on glue directory
Richard Weinberger <richard(a)nod.at>
ubifs: Correctly use tnc_next() in search_dh_cookie()
Alex Williamson <alex.williamson(a)redhat.com>
PCI: Always allow probing with driver_override
Xiaolei Li <xiaolei.li(a)mediatek.com>
mtd: rawnand: mtk: Fix wrongly assigned OOB buffer pointer issue
Douglas Anderson <dianders(a)chromium.org>
clk: rockchip: Don't yell about bad mmc phases when getting
Neil Armstrong <narmstrong(a)baylibre.com>
drm/meson: Add support for XBGR8888 & ABGR8888 formats
Suraj Jitindar Singh <sjitindarsingh(a)gmail.com>
powerpc: Add barrier_nospec to raw_copy_in_user()
Paul Burton <paul.burton(a)mips.com>
MIPS: VDSO: Use same -m%-float cflag as the kernel proper
Paul Burton <paul.burton(a)mips.com>
MIPS: VDSO: Prevent use of smp_processor_id()
Paolo Bonzini <pbonzini(a)redhat.com>
KVM: nVMX: handle page fault in vmread
Fuqian Huang <huangfq.daxian(a)gmail.com>
KVM: x86: work around leak of uninitialized stack contents
Thomas Huth <thuth(a)redhat.com>
KVM: s390: Do not leak kernel stack data in the KVM_S390_INTERRUPT ioctl
Yunfeng Ye <yeyunfeng(a)huawei.com>
genirq: Prevent NULL pointer dereference in resend_irqs()
Filipe Manana <fdmanana(a)suse.com>
Btrfs: fix assertion failure during fsync and use of stale transaction
Kent Gibson <warthog618(a)gmail.com>
gpio: fix line flag validation in lineevent_create
Kent Gibson <warthog618(a)gmail.com>
gpio: fix line flag validation in linehandle_create
Hans de Goede <hdegoede(a)redhat.com>
gpiolib: acpi: Add gpiolib_acpi_run_edge_events_on_boot option and blacklist
Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
Revert "MIPS: SiByte: Enable swiotlb for SWARM, LittleSur and BigSur"
Johannes Thumshirn <jthumshirn(a)suse.de>
btrfs: correctly validate compression type
David Sterba <dsterba(a)suse.com>
btrfs: compression: add helper for type to string conversion
Yang Yingliang <yangyingliang(a)huawei.com>
tun: fix use-after-free when register netdev failed
Xin Long <lucien.xin(a)gmail.com>
tipc: add NULL pointer check before calling kfree_rcu
Neal Cardwell <ncardwell(a)google.com>
tcp: fix tcp_ecn_withdraw_cwr() to clear TCP_ECN_QUEUE_CWR
Xin Long <lucien.xin(a)gmail.com>
sctp: use transport pf_retrans in sctp_do_8_2_transport_strike
Christophe JAILLET <christophe.jaillet(a)wanadoo.fr>
sctp: Fix the link time qualifier of 'sctp_ctrlsock_exit()'
Cong Wang <xiyou.wangcong(a)gmail.com>
sch_hhf: ensure quantum and hhf_non_hh_weight are non-zero
Stefan Chulski <stefanc(a)marvell.com>
net: phylink: Fix flow control resolution
Shmulik Ladkani <shmulik(a)metanetworks.com>
net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list
Subash Abhinov Kasiviswanathan <subashab(a)codeaurora.org>
net: Fix null de-reference of device refcount
Eric Biggers <ebiggers(a)google.com>
isdn/capi: check message length in capi_write()
Christophe JAILLET <christophe.jaillet(a)wanadoo.fr>
ipv6: Fix the link time qualifier of 'ping_v6_proc_exit_net()'
Bjørn Mork <bjorn(a)mork.no>
cdc_ether: fix rndis support for Mediatek based smartphones
Nicolas Dichtel <nicolas.dichtel(a)6wind.com>
bridge/mdb: remove wrong use of NLM_F_MULTI
-------------
Diffstat:
Makefile | 4 +-
arch/mips/Kconfig | 3 --
arch/mips/include/asm/smp.h | 12 +++++-
arch/mips/sibyte/common/Makefile | 1 -
arch/mips/sibyte/common/dma.c | 14 -------
arch/mips/vdso/Makefile | 4 +-
arch/powerpc/include/asm/uaccess.h | 1 +
arch/s390/kvm/interrupt.c | 10 +++++
arch/s390/kvm/kvm-s390.c | 2 +-
arch/x86/Makefile | 1 +
arch/x86/kvm/vmx.c | 7 +++-
arch/x86/kvm/x86.c | 7 ++++
drivers/base/core.c | 53 ++++++++++++++++++++++++++-
drivers/bluetooth/btusb.c | 5 ---
drivers/clk/rockchip/clk-mmc-phase.c | 4 +-
drivers/crypto/talitos.c | 67 +++++++++++++++++++++++++---------
drivers/firmware/ti_sci.c | 8 ++--
drivers/gpio/gpiolib-acpi.c | 42 +++++++++++++++++++--
drivers/gpio/gpiolib.c | 20 +++++++---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 ++-
drivers/gpu/drm/meson/meson_plane.c | 16 ++++++++
drivers/isdn/capi/capi.c | 10 ++++-
drivers/mtd/nand/mtk_nand.c | 21 +++++------
drivers/net/phy/phylink.c | 6 +--
drivers/net/tun.c | 16 +++++---
drivers/net/usb/cdc_ether.c | 13 +++++--
drivers/nvmem/core.c | 15 ++++++--
drivers/pci/pci-driver.c | 3 +-
drivers/platform/x86/pmc_atom.c | 8 ++++
fs/btrfs/compression.c | 31 ++++++++++++++++
fs/btrfs/compression.h | 3 ++
fs/btrfs/props.c | 6 +--
fs/btrfs/tree-log.c | 8 ++--
fs/ubifs/tnc.c | 16 +++++---
include/uapi/linux/isdn/capicmd.h | 1 +
kernel/irq/resend.c | 2 +
net/bridge/br_mdb.c | 2 +-
net/core/dev.c | 2 +
net/core/skbuff.c | 19 ++++++++++
net/ipv4/tcp_input.c | 2 +-
net/ipv6/ping.c | 2 +-
net/sched/sch_hhf.c | 2 +-
net/sctp/protocol.c | 2 +-
net/sctp/sm_sideeffect.c | 2 +-
net/tipc/name_distr.c | 3 +-
45 files changed, 366 insertions(+), 115 deletions(-)