Commit 57c5f4df authored by Xiubo Li's avatar Xiubo Li Committed by Greg Kroah-Hartman

uio: fix crash after the device is unregistered

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: default avatarHamish Martin <hamish.martin@alliedtelesis.co.nz>
Signed-off-by: default avatarXiubo Li <xiubli@redhat.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 543af586
...@@ -215,7 +215,20 @@ static ssize_t name_show(struct device *dev, ...@@ -215,7 +215,20 @@ static ssize_t name_show(struct device *dev,
struct device_attribute *attr, char *buf) struct device_attribute *attr, char *buf)
{ {
struct uio_device *idev = dev_get_drvdata(dev); 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); static DEVICE_ATTR_RO(name);
...@@ -223,7 +236,20 @@ static ssize_t version_show(struct device *dev, ...@@ -223,7 +236,20 @@ static ssize_t version_show(struct device *dev,
struct device_attribute *attr, char *buf) struct device_attribute *attr, char *buf)
{ {
struct uio_device *idev = dev_get_drvdata(dev); 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); static DEVICE_ATTR_RO(version);
...@@ -415,11 +441,15 @@ EXPORT_SYMBOL_GPL(uio_event_notify); ...@@ -415,11 +441,15 @@ EXPORT_SYMBOL_GPL(uio_event_notify);
static irqreturn_t uio_interrupt(int irq, void *dev_id) static irqreturn_t uio_interrupt(int irq, void *dev_id)
{ {
struct uio_device *idev = (struct uio_device *)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) if (ret == IRQ_HANDLED)
uio_event_notify(idev->info); uio_event_notify(idev->info);
mutex_unlock(&idev->info_lock);
return ret; return ret;
} }
...@@ -460,6 +490,12 @@ static int uio_open(struct inode *inode, struct file *filep) ...@@ -460,6 +490,12 @@ static int uio_open(struct inode *inode, struct file *filep)
filep->private_data = listener; filep->private_data = listener;
mutex_lock(&idev->info_lock); 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) if (idev->info && idev->info->open)
ret = idev->info->open(idev->info, inode); ret = idev->info->open(idev->info, inode);
mutex_unlock(&idev->info_lock); mutex_unlock(&idev->info_lock);
...@@ -590,6 +626,11 @@ static ssize_t uio_write(struct file *filep, const char __user *buf, ...@@ -590,6 +626,11 @@ static ssize_t uio_write(struct file *filep, const char __user *buf,
s32 irq_on; s32 irq_on;
mutex_lock(&idev->info_lock); mutex_lock(&idev->info_lock);
if (!idev->info) {
retval = -EINVAL;
goto out;
}
if (!idev->info || !idev->info->irq) { if (!idev->info || !idev->info->irq) {
retval = -EIO; retval = -EIO;
goto out; goto out;
...@@ -635,10 +676,20 @@ static vm_fault_t uio_vma_fault(struct vm_fault *vmf) ...@@ -635,10 +676,20 @@ static vm_fault_t uio_vma_fault(struct vm_fault *vmf)
struct page *page; struct page *page;
unsigned long offset; unsigned long offset;
void *addr; void *addr;
int ret = 0;
int mi;
int mi = uio_find_mem_index(vmf->vma); mutex_lock(&idev->info_lock);
if (mi < 0) if (!idev->info) {
return VM_FAULT_SIGBUS; 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 * We need to subtract mi because userspace uses offset = N*PAGE_SIZE
...@@ -653,7 +704,11 @@ static vm_fault_t uio_vma_fault(struct vm_fault *vmf) ...@@ -653,7 +704,11 @@ static vm_fault_t uio_vma_fault(struct vm_fault *vmf)
page = vmalloc_to_page(addr); page = vmalloc_to_page(addr);
get_page(page); get_page(page);
vmf->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 = { static const struct vm_operations_struct uio_logical_vm_ops = {
...@@ -678,6 +733,7 @@ static int uio_mmap_physical(struct vm_area_struct *vma) ...@@ -678,6 +733,7 @@ static int uio_mmap_physical(struct vm_area_struct *vma)
struct uio_device *idev = vma->vm_private_data; struct uio_device *idev = vma->vm_private_data;
int mi = uio_find_mem_index(vma); int mi = uio_find_mem_index(vma);
struct uio_mem *mem; struct uio_mem *mem;
if (mi < 0) if (mi < 0)
return -EINVAL; return -EINVAL;
mem = idev->info->mem + mi; mem = idev->info->mem + mi;
...@@ -719,30 +775,46 @@ static int uio_mmap(struct file *filep, struct vm_area_struct *vma) ...@@ -719,30 +775,46 @@ static int uio_mmap(struct file *filep, struct vm_area_struct *vma)
vma->vm_private_data = idev; vma->vm_private_data = idev;
mutex_lock(&idev->info_lock);
if (!idev->info) {
ret = -EINVAL;
goto out;
}
mi = uio_find_mem_index(vma); mi = uio_find_mem_index(vma);
if (mi < 0) if (mi < 0) {
return -EINVAL; ret = -EINVAL;
goto out;
}
requested_pages = vma_pages(vma); requested_pages = vma_pages(vma);
actual_pages = ((idev->info->mem[mi].addr & ~PAGE_MASK) actual_pages = ((idev->info->mem[mi].addr & ~PAGE_MASK)
+ idev->info->mem[mi].size + PAGE_SIZE -1) >> PAGE_SHIFT; + idev->info->mem[mi].size + PAGE_SIZE -1) >> PAGE_SHIFT;
if (requested_pages > actual_pages) if (requested_pages > actual_pages) {
return -EINVAL; ret = -EINVAL;
goto out;
}
if (idev->info->mmap) { if (idev->info->mmap) {
ret = idev->info->mmap(idev->info, vma); ret = idev->info->mmap(idev->info, vma);
return ret; goto out;
} }
switch (idev->info->mem[mi].memtype) { switch (idev->info->mem[mi].memtype) {
case UIO_MEM_PHYS: case UIO_MEM_PHYS:
return uio_mmap_physical(vma); ret = uio_mmap_physical(vma);
break;
case UIO_MEM_LOGICAL: case UIO_MEM_LOGICAL:
case UIO_MEM_VIRTUAL: case UIO_MEM_VIRTUAL:
return uio_mmap_logical(vma); ret = uio_mmap_logical(vma);
break;
default: default:
return -EINVAL; ret = -EINVAL;
} }
out:
mutex_unlock(&idev->info_lock);
return 0;
} }
static const struct file_operations uio_fops = { static const struct file_operations uio_fops = {
...@@ -932,12 +1004,12 @@ void uio_unregister_device(struct uio_info *info) ...@@ -932,12 +1004,12 @@ void uio_unregister_device(struct uio_info *info)
uio_free_minor(idev); uio_free_minor(idev);
mutex_lock(&idev->info_lock);
uio_dev_del_attributes(idev); uio_dev_del_attributes(idev);
if (info->irq && info->irq != UIO_IRQ_CUSTOM) if (info->irq && info->irq != UIO_IRQ_CUSTOM)
free_irq(info->irq, idev); free_irq(info->irq, idev);
mutex_lock(&idev->info_lock);
idev->info = NULL; idev->info = NULL;
mutex_unlock(&idev->info_lock); mutex_unlock(&idev->info_lock);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment