Backport uio fixes to 4.14, to fix use-after-free memory errors.
Changed __poll_t to unsigned int as the former not found in 4.14, and resolved some patch context conflicts.
Hailong Liu (1): uio: fix wrong return value from uio_mmap()
Hamish Martin (2): uio: Reduce return paths from uio_write() uio: Prevent device destruction while fds are open
Xiubo Li (5): uio: use request_threaded_irq instead uio: change to use the mutex lock instead of the spin lock uio: fix crash after the device is unregistered uio: fix possible circular locking dependency Revert "uio: use request_threaded_irq instead"
drivers/uio/uio.c | 206 ++++++++++++++++++++++++++++--------- include/linux/uio_driver.h | 4 +- 2 files changed, 163 insertions(+), 47 deletions(-)
From: Hamish Martin hamish.martin@alliedtelesis.co.nz
commit 81daa406c2cc97d85eef9409400404efc2a3f756 upstream.
Drive all return paths for uio_write() through a single block at the end of the function.
Signed-off-by: Hamish Martin hamish.martin@alliedtelesis.co.nz Reviewed-by: Chris Packham chris.packham@alliedtelesis.co.nz Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Tommi Rantala tommi.t.rantala@nokia.com --- drivers/uio/uio.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index 654579bc1e54..10f249628e79 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -570,20 +570,29 @@ static ssize_t uio_write(struct file *filep, const char __user *buf, ssize_t retval; s32 irq_on;
- if (!idev->info->irq) - return -EIO; + if (!idev->info->irq) { + retval = -EIO; + goto out; + }
- if (count != sizeof(s32)) - return -EINVAL; + if (count != sizeof(s32)) { + retval = -EINVAL; + goto out; + }
- if (!idev->info->irqcontrol) - return -ENOSYS; + if (!idev->info->irqcontrol) { + retval = -ENOSYS; + goto out; + }
- if (copy_from_user(&irq_on, buf, count)) - return -EFAULT; + if (copy_from_user(&irq_on, buf, count)) { + retval = -EFAULT; + goto out; + }
retval = idev->info->irqcontrol(idev->info, irq_on);
+out: return retval ? retval : sizeof(s32); }
From: Hamish Martin hamish.martin@alliedtelesis.co.nz
commit a93e7b331568227500186a465fee3c2cb5dffd1f upstream.
Prevent destruction of a uio_device while user space apps hold open file descriptors to that device. Further, access to the 'info' member of the struct uio_device is protected by spinlock. This is to ensure stale pointers to data not under control of the UIO subsystem are not dereferenced.
Signed-off-by: Hamish Martin hamish.martin@alliedtelesis.co.nz Reviewed-by: Chris Packham chris.packham@alliedtelesis.co.nz Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org [4.14 change __poll_t to unsigned int] Signed-off-by: Tommi Rantala tommi.t.rantala@nokia.com --- drivers/uio/uio.c | 98 ++++++++++++++++++++++++++++---------- include/linux/uio_driver.h | 4 +- 2 files changed, 75 insertions(+), 27 deletions(-)
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index 10f249628e79..288c4b977184 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -272,7 +272,7 @@ static int uio_dev_add_attributes(struct uio_device *idev) if (!map_found) { map_found = 1; idev->map_dir = kobject_create_and_add("maps", - &idev->dev->kobj); + &idev->dev.kobj); if (!idev->map_dir) { ret = -ENOMEM; goto err_map; @@ -301,7 +301,7 @@ static int uio_dev_add_attributes(struct uio_device *idev) if (!portio_found) { portio_found = 1; idev->portio_dir = kobject_create_and_add("portio", - &idev->dev->kobj); + &idev->dev.kobj); if (!idev->portio_dir) { ret = -ENOMEM; goto err_portio; @@ -344,7 +344,7 @@ static int uio_dev_add_attributes(struct uio_device *idev) kobject_put(&map->kobj); } kobject_put(idev->map_dir); - dev_err(idev->dev, "error creating sysfs files (%d)\n", ret); + dev_err(&idev->dev, "error creating sysfs files (%d)\n", ret); return ret; }
@@ -381,7 +381,7 @@ static int uio_get_minor(struct uio_device *idev) idev->minor = retval; retval = 0; } else if (retval == -ENOSPC) { - dev_err(idev->dev, "too many uio devices\n"); + dev_err(&idev->dev, "too many uio devices\n"); retval = -EINVAL; } mutex_unlock(&minor_lock); @@ -435,6 +435,7 @@ static int uio_open(struct inode *inode, struct file *filep) struct uio_device *idev; struct uio_listener *listener; int ret = 0; + unsigned long flags;
mutex_lock(&minor_lock); idev = idr_find(&uio_idr, iminor(inode)); @@ -444,9 +445,11 @@ static int uio_open(struct inode *inode, struct file *filep) goto out; }
+ get_device(&idev->dev); + if (!try_module_get(idev->owner)) { ret = -ENODEV; - goto out; + goto err_module_get; }
listener = kmalloc(sizeof(*listener), GFP_KERNEL); @@ -459,11 +462,13 @@ static int uio_open(struct inode *inode, struct file *filep) listener->event_count = atomic_read(&idev->event); filep->private_data = listener;
- if (idev->info->open) { + spin_lock_irqsave(&idev->info_lock, flags); + if (idev->info && idev->info->open) ret = idev->info->open(idev->info, inode); - if (ret) - goto err_infoopen; - } + spin_unlock_irqrestore(&idev->info_lock, flags); + if (ret) + goto err_infoopen; + return 0;
err_infoopen: @@ -472,6 +477,9 @@ static int uio_open(struct inode *inode, struct file *filep) err_alloc_listener: module_put(idev->owner);
+err_module_get: + put_device(&idev->dev); + out: return ret; } @@ -489,12 +497,16 @@ static int uio_release(struct inode *inode, struct file *filep) int ret = 0; struct uio_listener *listener = filep->private_data; struct uio_device *idev = listener->dev; + unsigned long flags;
- if (idev->info->release) + spin_lock_irqsave(&idev->info_lock, flags); + if (idev->info && idev->info->release) ret = idev->info->release(idev->info, inode); + spin_unlock_irqrestore(&idev->info_lock, flags);
module_put(idev->owner); kfree(listener); + put_device(&idev->dev); return ret; }
@@ -502,9 +514,16 @@ static unsigned int uio_poll(struct file *filep, poll_table *wait) { struct uio_listener *listener = filep->private_data; struct uio_device *idev = listener->dev; + unsigned int ret = 0; + unsigned long flags;
- if (!idev->info->irq) - return -EIO; + spin_lock_irqsave(&idev->info_lock, flags); + if (!idev->info || !idev->info->irq) + ret = -EIO; + spin_unlock_irqrestore(&idev->info_lock, flags); + + if (ret) + return ret;
poll_wait(filep, &idev->wait, wait); if (listener->event_count != atomic_read(&idev->event)) @@ -518,11 +537,17 @@ static ssize_t uio_read(struct file *filep, char __user *buf, struct uio_listener *listener = filep->private_data; struct uio_device *idev = listener->dev; DECLARE_WAITQUEUE(wait, current); - ssize_t retval; + ssize_t retval = 0; s32 event_count; + unsigned long flags;
- if (!idev->info->irq) - return -EIO; + spin_lock_irqsave(&idev->info_lock, flags); + if (!idev->info || !idev->info->irq) + retval = -EIO; + spin_unlock_irqrestore(&idev->info_lock, flags); + + if (retval) + return retval;
if (count != sizeof(s32)) return -EINVAL; @@ -569,8 +594,10 @@ static ssize_t uio_write(struct file *filep, const char __user *buf, struct uio_device *idev = listener->dev; ssize_t retval; s32 irq_on; + unsigned long flags;
- if (!idev->info->irq) { + spin_lock_irqsave(&idev->info_lock, flags); + if (!idev->info || !idev->info->irq) { retval = -EIO; goto out; } @@ -593,6 +620,7 @@ static ssize_t uio_write(struct file *filep, const char __user *buf, retval = idev->info->irqcontrol(idev->info, irq_on);
out: + spin_unlock_irqrestore(&idev->info_lock, flags); return retval ? retval : sizeof(s32); }
@@ -809,6 +837,13 @@ static void release_uio_class(void) uio_major_cleanup(); }
+static void uio_device_release(struct device *dev) +{ + struct uio_device *idev = dev_get_drvdata(dev); + + kfree(idev); +} + /** * uio_register_device - register a new userspace IO device * @owner: module that creates the new device @@ -832,13 +867,14 @@ int __uio_register_device(struct module *owner,
info->uio_dev = NULL;
- idev = devm_kzalloc(parent, sizeof(*idev), GFP_KERNEL); + idev = kzalloc(sizeof(*idev), GFP_KERNEL); if (!idev) { return -ENOMEM; }
idev->owner = owner; idev->info = info; + spin_lock_init(&idev->info_lock); init_waitqueue_head(&idev->wait); atomic_set(&idev->event, 0);
@@ -846,14 +882,19 @@ int __uio_register_device(struct module *owner, if (ret) return ret;
- idev->dev = device_create(&uio_class, parent, - MKDEV(uio_major, idev->minor), idev, - "uio%d", idev->minor); - if (IS_ERR(idev->dev)) { - printk(KERN_ERR "UIO: device register failed\n"); - ret = PTR_ERR(idev->dev); + idev->dev.devt = MKDEV(uio_major, idev->minor); + idev->dev.class = &uio_class; + idev->dev.parent = parent; + idev->dev.release = uio_device_release; + dev_set_drvdata(&idev->dev, idev); + + ret = dev_set_name(&idev->dev, "uio%d", idev->minor); + if (ret) + goto err_device_create; + + ret = device_register(&idev->dev); + if (ret) goto err_device_create; - }
ret = uio_dev_add_attributes(idev); if (ret) @@ -883,7 +924,7 @@ int __uio_register_device(struct module *owner, err_request_irq: uio_dev_del_attributes(idev); err_uio_dev_add_attributes: - device_destroy(&uio_class, MKDEV(uio_major, idev->minor)); + device_unregister(&idev->dev); err_device_create: uio_free_minor(idev); return ret; @@ -898,6 +939,7 @@ EXPORT_SYMBOL_GPL(__uio_register_device); void uio_unregister_device(struct uio_info *info) { struct uio_device *idev; + unsigned long flags;
if (!info || !info->uio_dev) return; @@ -911,7 +953,11 @@ void uio_unregister_device(struct uio_info *info) if (info->irq && info->irq != UIO_IRQ_CUSTOM) free_irq(info->irq, idev);
- device_destroy(&uio_class, MKDEV(uio_major, idev->minor)); + spin_lock_irqsave(&idev->info_lock, flags); + idev->info = NULL; + spin_unlock_irqrestore(&idev->info_lock, flags); + + device_unregister(&idev->dev);
return; } diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h index 3c85c81b0027..6c5f2074e14f 100644 --- a/include/linux/uio_driver.h +++ b/include/linux/uio_driver.h @@ -14,6 +14,7 @@ #ifndef _UIO_DRIVER_H_ #define _UIO_DRIVER_H_
+#include <linux/device.h> #include <linux/fs.h> #include <linux/interrupt.h>
@@ -68,12 +69,13 @@ struct uio_port {
struct uio_device { struct module *owner; - struct device *dev; + struct device dev; int minor; atomic_t event; struct fasync_struct *async_queue; wait_queue_head_t wait; struct uio_info *info; + spinlock_t info_lock; struct kobject *map_dir; struct kobject *portio_dir; };
From: Xiubo Li xiubli@redhat.com
commit 9421e45f5ff3d558cf8b75a8cc0824530caf3453 upstream.
Prepraing for changing to use mutex lock.
Signed-off-by: Xiubo Li xiubli@redhat.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Tommi Rantala tommi.t.rantala@nokia.com --- drivers/uio/uio.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index 288c4b977184..c97945a3f572 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -911,8 +911,9 @@ int __uio_register_device(struct module *owner, * FDs at the time of unregister and therefore may not be * freed until they are released. */ - ret = request_irq(info->irq, uio_interrupt, - info->irq_flags, info->name, idev); + ret = request_threaded_irq(info->irq, NULL, uio_interrupt, + info->irq_flags, info->name, idev); + if (ret) { info->uio_dev = NULL; goto err_request_irq;
From: Xiubo Li xiubli@redhat.com
commit 543af5861f41af0a5d2432f6fb5976af50f9cee5 upstream.
We are hitting a regression with the following commit:
commit a93e7b331568227500186a465fee3c2cb5dffd1f Author: Hamish Martin hamish.martin@alliedtelesis.co.nz Date: Mon May 14 13:32:23 2018 +1200
uio: Prevent device destruction while fds are open
The problem is the addition of spin_lock_irqsave in uio_write. This leads to hitting uio_write -> copy_from_user -> _copy_from_user -> might_fault and the logs filling up with sleeping warnings.
I also noticed some uio drivers allocate memory, sleep, grab mutexes from callouts like open() and release and uio is now doing spin_lock_irqsave while calling them.
Reported-by: Mike Christie mchristi@redhat.com CC: Hamish Martin hamish.martin@alliedtelesis.co.nz Reviewed-by: Hamish Martin hamish.martin@alliedtelesis.co.nz Signed-off-by: Xiubo Li xiubli@redhat.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Tommi Rantala tommi.t.rantala@nokia.com --- drivers/uio/uio.c | 32 +++++++++++++------------------- include/linux/uio_driver.h | 2 +- 2 files changed, 14 insertions(+), 20 deletions(-)
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index c97945a3f572..4441235a56cc 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -435,7 +435,6 @@ static int uio_open(struct inode *inode, struct file *filep) struct uio_device *idev; struct uio_listener *listener; int ret = 0; - unsigned long flags;
mutex_lock(&minor_lock); idev = idr_find(&uio_idr, iminor(inode)); @@ -462,10 +461,10 @@ static int uio_open(struct inode *inode, struct file *filep) listener->event_count = atomic_read(&idev->event); filep->private_data = listener;
- spin_lock_irqsave(&idev->info_lock, flags); + mutex_lock(&idev->info_lock); if (idev->info && idev->info->open) ret = idev->info->open(idev->info, inode); - spin_unlock_irqrestore(&idev->info_lock, flags); + mutex_unlock(&idev->info_lock); if (ret) goto err_infoopen;
@@ -497,12 +496,11 @@ static int uio_release(struct inode *inode, struct file *filep) int ret = 0; struct uio_listener *listener = filep->private_data; struct uio_device *idev = listener->dev; - unsigned long flags;
- spin_lock_irqsave(&idev->info_lock, flags); + mutex_lock(&idev->info_lock); if (idev->info && idev->info->release) ret = idev->info->release(idev->info, inode); - spin_unlock_irqrestore(&idev->info_lock, flags); + mutex_unlock(&idev->info_lock);
module_put(idev->owner); kfree(listener); @@ -515,12 +513,11 @@ static unsigned int uio_poll(struct file *filep, poll_table *wait) struct uio_listener *listener = filep->private_data; struct uio_device *idev = listener->dev; unsigned int ret = 0; - unsigned long flags;
- spin_lock_irqsave(&idev->info_lock, flags); + mutex_lock(&idev->info_lock); if (!idev->info || !idev->info->irq) ret = -EIO; - spin_unlock_irqrestore(&idev->info_lock, flags); + mutex_unlock(&idev->info_lock);
if (ret) return ret; @@ -539,12 +536,11 @@ static ssize_t uio_read(struct file *filep, char __user *buf, DECLARE_WAITQUEUE(wait, current); ssize_t retval = 0; s32 event_count; - unsigned long flags;
- spin_lock_irqsave(&idev->info_lock, flags); + mutex_lock(&idev->info_lock); if (!idev->info || !idev->info->irq) retval = -EIO; - spin_unlock_irqrestore(&idev->info_lock, flags); + mutex_unlock(&idev->info_lock);
if (retval) return retval; @@ -594,9 +590,8 @@ static ssize_t uio_write(struct file *filep, const char __user *buf, struct uio_device *idev = listener->dev; ssize_t retval; s32 irq_on; - unsigned long flags;
- spin_lock_irqsave(&idev->info_lock, flags); + mutex_lock(&idev->info_lock); if (!idev->info || !idev->info->irq) { retval = -EIO; goto out; @@ -620,7 +615,7 @@ static ssize_t uio_write(struct file *filep, const char __user *buf, retval = idev->info->irqcontrol(idev->info, irq_on);
out: - spin_unlock_irqrestore(&idev->info_lock, flags); + mutex_unlock(&idev->info_lock); return retval ? retval : sizeof(s32); }
@@ -874,7 +869,7 @@ int __uio_register_device(struct module *owner,
idev->owner = owner; idev->info = info; - spin_lock_init(&idev->info_lock); + mutex_init(&idev->info_lock); init_waitqueue_head(&idev->wait); atomic_set(&idev->event, 0);
@@ -940,7 +935,6 @@ EXPORT_SYMBOL_GPL(__uio_register_device); void uio_unregister_device(struct uio_info *info) { struct uio_device *idev; - unsigned long flags;
if (!info || !info->uio_dev) return; @@ -954,9 +948,9 @@ void uio_unregister_device(struct uio_info *info) if (info->irq && info->irq != UIO_IRQ_CUSTOM) free_irq(info->irq, idev);
- spin_lock_irqsave(&idev->info_lock, flags); + mutex_lock(&idev->info_lock); idev->info = NULL; - spin_unlock_irqrestore(&idev->info_lock, flags); + mutex_unlock(&idev->info_lock);
device_unregister(&idev->dev);
diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h index 6c5f2074e14f..6f8b68cd460f 100644 --- a/include/linux/uio_driver.h +++ b/include/linux/uio_driver.h @@ -75,7 +75,7 @@ struct uio_device { struct fasync_struct *async_queue; wait_queue_head_t wait; struct uio_info *info; - spinlock_t info_lock; + struct mutex info_lock; struct kobject *map_dir; struct kobject *portio_dir; };
From: Xiubo Li xiubli@redhat.com
commit 57c5f4df0a5a0ee83df799991251e2ee93a5e4e9 upstream.
For the target_core_user use case, after the device is unregistered it maybe still opened in user space, then the kernel will crash, like:
[ 251.163692] BUG: unable to handle kernel NULL pointer dereference at 0000000000000008 [ 251.163820] IP: [<ffffffffc0736213>] show_name+0x23/0x40 [uio] [ 251.163965] PGD 8000000062694067 PUD 62696067 PMD 0 [ 251.164097] Oops: 0000 [#1] SMP ... [ 251.165605] e1000 mptscsih mptbase drm_panel_orientation_quirks dm_mirror dm_region_hash dm_log dm_mod [ 251.166014] CPU: 0 PID: 13380 Comm: tcmu-runner Kdump: loaded Not tainted 3.10.0-916.el7.test.x86_64 #1 [ 251.166381] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/19/2017 [ 251.166747] task: ffff971eb91db0c0 ti: ffff971e9e384000 task.ti: ffff971e9e384000 [ 251.167137] RIP: 0010:[<ffffffffc0736213>] [<ffffffffc0736213>] show_name+0x23/0x40 [uio] [ 251.167563] RSP: 0018:ffff971e9e387dc8 EFLAGS: 00010282 [ 251.167978] RAX: 0000000000000000 RBX: ffff971e9e3f8000 RCX: ffff971eb8368d98 [ 251.168408] RDX: ffff971e9e3f8000 RSI: ffffffffc0738084 RDI: ffff971e9e3f8000 [ 251.168856] RBP: ffff971e9e387dd0 R08: ffff971eb8bc0018 R09: 0000000000000000 [ 251.169296] R10: 0000000000001000 R11: ffffffffa09d444d R12: ffffffffa1076e80 [ 251.169750] R13: ffff971e9e387f18 R14: 0000000000000001 R15: ffff971e9cfb1c80 [ 251.170213] FS: 00007ff37d175880(0000) GS:ffff971ebb600000(0000) knlGS:0000000000000000 [ 251.170693] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 251.171248] CR2: 0000000000000008 CR3: 00000000001f6000 CR4: 00000000003607f0 [ 251.172071] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 251.172640] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 251.173236] Call Trace: [ 251.173789] [<ffffffffa0c9b2d3>] dev_attr_show+0x23/0x60 [ 251.174356] [<ffffffffa0f561b2>] ? mutex_lock+0x12/0x2f [ 251.174892] [<ffffffffa0ac6d9f>] sysfs_kf_seq_show+0xcf/0x1f0 [ 251.175433] [<ffffffffa0ac54e6>] kernfs_seq_show+0x26/0x30 [ 251.175981] [<ffffffffa0a63be0>] seq_read+0x110/0x3f0 [ 251.176609] [<ffffffffa0ac5d45>] kernfs_fop_read+0xf5/0x160 [ 251.177158] [<ffffffffa0a3d3af>] vfs_read+0x9f/0x170 [ 251.177707] [<ffffffffa0a3e27f>] SyS_read+0x7f/0xf0 [ 251.178268] [<ffffffffa0f648af>] system_call_fastpath+0x1c/0x21 [ 251.178823] Code: 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 48 89 e5 53 48 89 d3 e8 7e 96 56 e0 48 8b 80 d8 02 00 00 48 89 df 48 c7 c6 84 80 73 c0 <48> 8b 50 08 31 c0 e8 e2 67 44 e0 5b 48 98 5d c3 0f 1f 00 66 2e [ 251.180115] RIP [<ffffffffc0736213>] show_name+0x23/0x40 [uio] [ 251.180820] RSP <ffff971e9e387dc8> [ 251.181473] CR2: 0000000000000008
CC: Hamish Martin hamish.martin@alliedtelesis.co.nz CC: Mike Christie mchristi@redhat.com Reviewed-by: Hamish Martin hamish.martin@alliedtelesis.co.nz Signed-off-by: Xiubo Li xiubli@redhat.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Tommi Rantala tommi.t.rantala@nokia.com --- drivers/uio/uio.c | 104 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 88 insertions(+), 16 deletions(-)
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index 4441235a56cc..262610192755 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -215,7 +215,20 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr, char *buf) { struct uio_device *idev = dev_get_drvdata(dev); - return sprintf(buf, "%s\n", idev->info->name); + int ret; + + mutex_lock(&idev->info_lock); + if (!idev->info) { + ret = -EINVAL; + dev_err(dev, "the device has been unregistered\n"); + goto out; + } + + ret = sprintf(buf, "%s\n", idev->info->name); + +out: + mutex_unlock(&idev->info_lock); + return ret; } static DEVICE_ATTR_RO(name);
@@ -223,7 +236,20 @@ static ssize_t version_show(struct device *dev, struct device_attribute *attr, char *buf) { struct uio_device *idev = dev_get_drvdata(dev); - return sprintf(buf, "%s\n", idev->info->version); + int ret; + + mutex_lock(&idev->info_lock); + if (!idev->info) { + ret = -EINVAL; + dev_err(dev, "the device has been unregistered\n"); + goto out; + } + + ret = sprintf(buf, "%s\n", idev->info->version); + +out: + mutex_unlock(&idev->info_lock); + return ret; } static DEVICE_ATTR_RO(version);
@@ -417,11 +443,15 @@ EXPORT_SYMBOL_GPL(uio_event_notify); static irqreturn_t uio_interrupt(int irq, void *dev_id) { struct uio_device *idev = (struct uio_device *)dev_id; - irqreturn_t ret = idev->info->handler(irq, idev->info); + irqreturn_t ret; + + mutex_lock(&idev->info_lock);
+ ret = idev->info->handler(irq, idev->info); if (ret == IRQ_HANDLED) uio_event_notify(idev->info);
+ mutex_unlock(&idev->info_lock); return ret; }
@@ -462,6 +492,12 @@ static int uio_open(struct inode *inode, struct file *filep) filep->private_data = listener;
mutex_lock(&idev->info_lock); + if (!idev->info) { + mutex_unlock(&idev->info_lock); + ret = -EINVAL; + goto err_alloc_listener; + } + if (idev->info && idev->info->open) ret = idev->info->open(idev->info, inode); mutex_unlock(&idev->info_lock); @@ -592,6 +628,11 @@ static ssize_t uio_write(struct file *filep, const char __user *buf, s32 irq_on;
mutex_lock(&idev->info_lock); + if (!idev->info) { + retval = -EINVAL; + goto out; + } + if (!idev->info || !idev->info->irq) { retval = -EIO; goto out; @@ -637,10 +678,20 @@ static int uio_vma_fault(struct vm_fault *vmf) struct page *page; unsigned long offset; void *addr; + int ret = 0; + int mi;
- int mi = uio_find_mem_index(vmf->vma); - if (mi < 0) - return VM_FAULT_SIGBUS; + mutex_lock(&idev->info_lock); + if (!idev->info) { + ret = VM_FAULT_SIGBUS; + goto out; + } + + mi = uio_find_mem_index(vmf->vma); + if (mi < 0) { + ret = VM_FAULT_SIGBUS; + goto out; + }
/* * We need to subtract mi because userspace uses offset = N*PAGE_SIZE @@ -655,7 +706,11 @@ static int uio_vma_fault(struct vm_fault *vmf) page = vmalloc_to_page(addr); get_page(page); vmf->page = page; - return 0; + +out: + mutex_unlock(&idev->info_lock); + + return ret; }
static const struct vm_operations_struct uio_logical_vm_ops = { @@ -680,6 +735,7 @@ static int uio_mmap_physical(struct vm_area_struct *vma) struct uio_device *idev = vma->vm_private_data; int mi = uio_find_mem_index(vma); struct uio_mem *mem; + if (mi < 0) return -EINVAL; mem = idev->info->mem + mi; @@ -721,30 +777,46 @@ static int uio_mmap(struct file *filep, struct vm_area_struct *vma)
vma->vm_private_data = idev;
+ mutex_lock(&idev->info_lock); + if (!idev->info) { + ret = -EINVAL; + goto out; + } + mi = uio_find_mem_index(vma); - if (mi < 0) - return -EINVAL; + if (mi < 0) { + ret = -EINVAL; + goto out; + }
requested_pages = vma_pages(vma); actual_pages = ((idev->info->mem[mi].addr & ~PAGE_MASK) + idev->info->mem[mi].size + PAGE_SIZE -1) >> PAGE_SHIFT; - if (requested_pages > actual_pages) - return -EINVAL; + if (requested_pages > actual_pages) { + ret = -EINVAL; + goto out; + }
if (idev->info->mmap) { ret = idev->info->mmap(idev->info, vma); - return ret; + goto out; }
switch (idev->info->mem[mi].memtype) { case UIO_MEM_PHYS: - return uio_mmap_physical(vma); + ret = uio_mmap_physical(vma); + break; case UIO_MEM_LOGICAL: case UIO_MEM_VIRTUAL: - return uio_mmap_logical(vma); + ret = uio_mmap_logical(vma); + break; default: - return -EINVAL; + ret = -EINVAL; } + +out: + mutex_unlock(&idev->info_lock); + return 0; }
static const struct file_operations uio_fops = { @@ -943,12 +1015,12 @@ void uio_unregister_device(struct uio_info *info)
uio_free_minor(idev);
+ mutex_lock(&idev->info_lock); uio_dev_del_attributes(idev);
if (info->irq && info->irq != UIO_IRQ_CUSTOM) free_irq(info->irq, idev);
- mutex_lock(&idev->info_lock); idev->info = NULL; mutex_unlock(&idev->info_lock);
From: Xiubo Li xiubli@redhat.com
commit b34e9a15b37b8ddbf06a4da142b0c39c74211eb4 upstream.
The call trace: XXX/1910 is trying to acquire lock: (&mm->mmap_sem){++++++}, at: [<ffffffff97008c87>] might_fault+0x57/0xb0
but task is already holding lock: (&idev->info_lock){+.+...}, at: [<ffffffffc0638a06>] uio_write+0x46/0x130 [uio]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&idev->info_lock){+.+...}: [<ffffffff96f31fc9>] lock_acquire+0x99/0x1e0 [<ffffffff975edad3>] mutex_lock_nested+0x93/0x410 [<ffffffffc063873d>] uio_mmap+0x2d/0x170 [uio] [<ffffffff97016b58>] mmap_region+0x428/0x650 [<ffffffff97017138>] do_mmap+0x3b8/0x4e0 [<ffffffff96ffaba3>] vm_mmap_pgoff+0xd3/0x120 [<ffffffff97015261>] SyS_mmap_pgoff+0x1f1/0x270 [<ffffffff96e387c2>] SyS_mmap+0x22/0x30 [<ffffffff975ff315>] system_call_fastpath+0x1c/0x21
-> #0 (&mm->mmap_sem){++++++}: [<ffffffff96f30e9c>] __lock_acquire+0xdac/0x15f0 [<ffffffff96f31fc9>] lock_acquire+0x99/0x1e0 [<ffffffff97008cb4>] might_fault+0x84/0xb0 [<ffffffffc0638a74>] uio_write+0xb4/0x130 [uio] [<ffffffff9706ffa3>] vfs_write+0xc3/0x1f0 [<ffffffff97070e2a>] SyS_write+0x8a/0x100 [<ffffffff975ff315>] system_call_fastpath+0x1c/0x21
other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&idev->info_lock); lock(&mm->mmap_sem); lock(&idev->info_lock); lock(&mm->mmap_sem);
*** DEADLOCK *** 1 lock held by XXX/1910: #0: (&idev->info_lock){+.+...}, at: [<ffffffffc0638a06>] uio_write+0x46/0x130 [uio]
stack backtrace: CPU: 0 PID: 1910 Comm: XXX Kdump: loaded Not tainted #1 Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/19/2017 Call Trace: [<ffffffff975e9211>] dump_stack+0x19/0x1b [<ffffffff975e260a>] print_circular_bug+0x1f9/0x207 [<ffffffff96f2f6a7>] check_prevs_add+0x957/0x960 [<ffffffff96f30e9c>] __lock_acquire+0xdac/0x15f0 [<ffffffff96f2fb19>] ? mark_held_locks+0xb9/0x140 [<ffffffff96f31fc9>] lock_acquire+0x99/0x1e0 [<ffffffff97008c87>] ? might_fault+0x57/0xb0 [<ffffffff97008cb4>] might_fault+0x84/0xb0 [<ffffffff97008c87>] ? might_fault+0x57/0xb0 [<ffffffffc0638a74>] uio_write+0xb4/0x130 [uio] [<ffffffff9706ffa3>] vfs_write+0xc3/0x1f0 [<ffffffff9709349c>] ? fget_light+0xfc/0x510 [<ffffffff97070e2a>] SyS_write+0x8a/0x100 [<ffffffff975ff315>] system_call_fastpath+0x1c/0x21
Signed-off-by: Xiubo Li xiubli@redhat.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Tommi Rantala tommi.t.rantala@nokia.com --- drivers/uio/uio.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index fed2d8fa4d4d..4e0cb7cdf739 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -627,6 +627,12 @@ static ssize_t uio_write(struct file *filep, const char __user *buf, ssize_t retval; s32 irq_on;
+ if (count != sizeof(s32)) + return -EINVAL; + + if (copy_from_user(&irq_on, buf, count)) + return -EFAULT; + mutex_lock(&idev->info_lock); if (!idev->info) { retval = -EINVAL; @@ -638,21 +644,11 @@ static ssize_t uio_write(struct file *filep, const char __user *buf, goto out; }
- if (count != sizeof(s32)) { - retval = -EINVAL; - goto out; - } - if (!idev->info->irqcontrol) { retval = -ENOSYS; goto out; }
- if (copy_from_user(&irq_on, buf, count)) { - retval = -EFAULT; - goto out; - } - retval = idev->info->irqcontrol(idev->info, irq_on);
out:
From: Hailong Liu liu.hailong6@zte.com.cn
commit e7de2590f18a272e63732b9d519250d1b522b2c4 upstream.
uio_mmap has multiple fail paths to set return value to nonzero then goto out. However, it always returns *0* from the *out* at end, and this will mislead callers who check the return value of this function.
Fixes: 57c5f4df0a5a0ee ("uio: fix crash after the device is unregistered") CC: Xiubo Li xiubli@redhat.com Signed-off-by: Hailong Liu liu.hailong6@zte.com.cn Cc: stable stable@vger.kernel.org Signed-off-by: Jiang Biao jiang.biao2@zte.com.cn Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Tommi Rantala tommi.t.rantala@nokia.com --- drivers/uio/uio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index 262610192755..fed2d8fa4d4d 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -816,7 +816,7 @@ static int uio_mmap(struct file *filep, struct vm_area_struct *vma)
out: mutex_unlock(&idev->info_lock); - return 0; + return ret; }
static const struct file_operations uio_fops = {
From: Xiubo Li xiubli@redhat.com
commit 3d27c4de8d4fb2d4099ff324671792aa2578c6f9 upstream.
Since mutex lock in irq hanler is useless currently, here will remove it together with it.
This reverts commit 9421e45f5ff3d558cf8b75a8cc0824530caf3453.
Reported-by: james.r.harris@intel.com CC: Ahsan Atta ahsan.atta@intel.com Signed-off-by: Xiubo Li xiubli@redhat.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Tommi Rantala tommi.t.rantala@nokia.com --- drivers/uio/uio.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index 4e0cb7cdf739..fb5c9701b1fb 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -445,13 +445,10 @@ static irqreturn_t uio_interrupt(int irq, void *dev_id) struct uio_device *idev = (struct uio_device *)dev_id; irqreturn_t ret;
- mutex_lock(&idev->info_lock); - ret = idev->info->handler(irq, idev->info); if (ret == IRQ_HANDLED) uio_event_notify(idev->info);
- mutex_unlock(&idev->info_lock); return ret; }
@@ -974,9 +971,8 @@ int __uio_register_device(struct module *owner, * FDs at the time of unregister and therefore may not be * freed until they are released. */ - ret = request_threaded_irq(info->irq, NULL, uio_interrupt, - info->irq_flags, info->name, idev); - + ret = request_irq(info->irq, uio_interrupt, + info->irq_flags, info->name, idev); if (ret) { info->uio_dev = NULL; goto err_request_irq;
On Wed, Feb 13, 2019 at 04:29:12PM +0000, Rantala, Tommi T. (Nokia - FI/Espoo) wrote:
Backport uio fixes to 4.14, to fix use-after-free memory errors.
Changed __poll_t to unsigned int as the former not found in 4.14, and resolved some patch context conflicts.
Thanks for this, all now queued up.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Rantala, Tommi T. (Nokia - FI/Espoo)