Skip to content

Commit 543af58

Browse files
lxbszgregkh
authored andcommitted
uio: change to use the mutex lock instead of the spin lock
We are hitting a regression with the following commit: commit a93e7b3 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>
1 parent 9421e45 commit 543af58

File tree

2 files changed

+14
-20
lines changed

2 files changed

+14
-20
lines changed

drivers/uio/uio.c

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,6 @@ static int uio_open(struct inode *inode, struct file *filep)
433433
struct uio_device *idev;
434434
struct uio_listener *listener;
435435
int ret = 0;
436-
unsigned long flags;
437436

438437
mutex_lock(&minor_lock);
439438
idev = idr_find(&uio_idr, iminor(inode));
@@ -460,10 +459,10 @@ static int uio_open(struct inode *inode, struct file *filep)
460459
listener->event_count = atomic_read(&idev->event);
461460
filep->private_data = listener;
462461

463-
spin_lock_irqsave(&idev->info_lock, flags);
462+
mutex_lock(&idev->info_lock);
464463
if (idev->info && idev->info->open)
465464
ret = idev->info->open(idev->info, inode);
466-
spin_unlock_irqrestore(&idev->info_lock, flags);
465+
mutex_unlock(&idev->info_lock);
467466
if (ret)
468467
goto err_infoopen;
469468

@@ -495,12 +494,11 @@ static int uio_release(struct inode *inode, struct file *filep)
495494
int ret = 0;
496495
struct uio_listener *listener = filep->private_data;
497496
struct uio_device *idev = listener->dev;
498-
unsigned long flags;
499497

500-
spin_lock_irqsave(&idev->info_lock, flags);
498+
mutex_lock(&idev->info_lock);
501499
if (idev->info && idev->info->release)
502500
ret = idev->info->release(idev->info, inode);
503-
spin_unlock_irqrestore(&idev->info_lock, flags);
501+
mutex_unlock(&idev->info_lock);
504502

505503
module_put(idev->owner);
506504
kfree(listener);
@@ -513,12 +511,11 @@ static __poll_t uio_poll(struct file *filep, poll_table *wait)
513511
struct uio_listener *listener = filep->private_data;
514512
struct uio_device *idev = listener->dev;
515513
__poll_t ret = 0;
516-
unsigned long flags;
517514

518-
spin_lock_irqsave(&idev->info_lock, flags);
515+
mutex_lock(&idev->info_lock);
519516
if (!idev->info || !idev->info->irq)
520517
ret = -EIO;
521-
spin_unlock_irqrestore(&idev->info_lock, flags);
518+
mutex_unlock(&idev->info_lock);
522519

523520
if (ret)
524521
return ret;
@@ -537,12 +534,11 @@ static ssize_t uio_read(struct file *filep, char __user *buf,
537534
DECLARE_WAITQUEUE(wait, current);
538535
ssize_t retval = 0;
539536
s32 event_count;
540-
unsigned long flags;
541537

542-
spin_lock_irqsave(&idev->info_lock, flags);
538+
mutex_lock(&idev->info_lock);
543539
if (!idev->info || !idev->info->irq)
544540
retval = -EIO;
545-
spin_unlock_irqrestore(&idev->info_lock, flags);
541+
mutex_unlock(&idev->info_lock);
546542

547543
if (retval)
548544
return retval;
@@ -592,9 +588,8 @@ static ssize_t uio_write(struct file *filep, const char __user *buf,
592588
struct uio_device *idev = listener->dev;
593589
ssize_t retval;
594590
s32 irq_on;
595-
unsigned long flags;
596591

597-
spin_lock_irqsave(&idev->info_lock, flags);
592+
mutex_lock(&idev->info_lock);
598593
if (!idev->info || !idev->info->irq) {
599594
retval = -EIO;
600595
goto out;
@@ -618,7 +613,7 @@ static ssize_t uio_write(struct file *filep, const char __user *buf,
618613
retval = idev->info->irqcontrol(idev->info, irq_on);
619614

620615
out:
621-
spin_unlock_irqrestore(&idev->info_lock, flags);
616+
mutex_unlock(&idev->info_lock);
622617
return retval ? retval : sizeof(s32);
623618
}
624619

@@ -865,7 +860,7 @@ int __uio_register_device(struct module *owner,
865860

866861
idev->owner = owner;
867862
idev->info = info;
868-
spin_lock_init(&idev->info_lock);
863+
mutex_init(&idev->info_lock);
869864
init_waitqueue_head(&idev->wait);
870865
atomic_set(&idev->event, 0);
871866

@@ -929,7 +924,6 @@ EXPORT_SYMBOL_GPL(__uio_register_device);
929924
void uio_unregister_device(struct uio_info *info)
930925
{
931926
struct uio_device *idev;
932-
unsigned long flags;
933927

934928
if (!info || !info->uio_dev)
935929
return;
@@ -943,9 +937,9 @@ void uio_unregister_device(struct uio_info *info)
943937
if (info->irq && info->irq != UIO_IRQ_CUSTOM)
944938
free_irq(info->irq, idev);
945939

946-
spin_lock_irqsave(&idev->info_lock, flags);
940+
mutex_lock(&idev->info_lock);
947941
idev->info = NULL;
948-
spin_unlock_irqrestore(&idev->info_lock, flags);
942+
mutex_unlock(&idev->info_lock);
949943

950944
device_unregister(&idev->dev);
951945

include/linux/uio_driver.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ struct uio_device {
7575
struct fasync_struct *async_queue;
7676
wait_queue_head_t wait;
7777
struct uio_info *info;
78-
spinlock_t info_lock;
78+
struct mutex info_lock;
7979
struct kobject *map_dir;
8080
struct kobject *portio_dir;
8181
};

0 commit comments

Comments
 (0)