diff options
author | Stefan Richter <stefanr@s5r6.in-berlin.de> | 2008-08-15 18:11:48 -0400 |
---|---|---|
committer | Stefan Richter <stefanr@s5r6.in-berlin.de> | 2008-10-15 16:21:08 -0400 |
commit | 10963ea1bd966ba46a46178c4d6abcdf3c23538d (patch) | |
tree | d11eb5adbca18c9570c28fff4dffe0f7d4d1da1c | |
parent | ed6ffd08084c68e9c3911e27706dec9d4c9a4175 (diff) |
ieee1394: raw1394: replace BKL by local mutex, make ioctl() and mmap() thread-safe
This removes the last usage of the Big Kernel Lock from the ieee1394
stack, i.e. from raw1394's (unlocked_)ioctl and compat_ioctl.
The ioctl()s don't need to take the BKL, but they need to be serialized
per struct file *. In particular, accesses to ->iso_state need to be
serial. We simply use a blocking mutex for this purpose because
libraw1394 does not use O_NONBLOCK. In practice, there is no lock
contention anyway because most if not all libraw1394 clients use a
libraw1394 handle only in a single thread.
mmap() also accesses ->iso_state. Until now this was unprotected
against concurrent changes by ioctls. Fix this bug while we are at it.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
-rw-r--r-- | drivers/ieee1394/raw1394-private.h | 1 | ||||
-rw-r--r-- | drivers/ieee1394/raw1394.c | 23 |
2 files changed, 18 insertions, 6 deletions
diff --git a/drivers/ieee1394/raw1394-private.h b/drivers/ieee1394/raw1394-private.h index a06aaad5b448..7a225a405987 100644 --- a/drivers/ieee1394/raw1394-private.h +++ b/drivers/ieee1394/raw1394-private.h | |||
@@ -22,6 +22,7 @@ enum raw1394_iso_state { RAW1394_ISO_INACTIVE = 0, | |||
22 | struct file_info { | 22 | struct file_info { |
23 | struct list_head list; | 23 | struct list_head list; |
24 | 24 | ||
25 | struct mutex state_mutex; | ||
25 | enum { opened, initialized, connected } state; | 26 | enum { opened, initialized, connected } state; |
26 | unsigned int protocol_version; | 27 | unsigned int protocol_version; |
27 | 28 | ||
diff --git a/drivers/ieee1394/raw1394.c b/drivers/ieee1394/raw1394.c index 6fa9e4a21840..975ed7062aa1 100644 --- a/drivers/ieee1394/raw1394.c +++ b/drivers/ieee1394/raw1394.c | |||
@@ -34,6 +34,7 @@ | |||
34 | #include <linux/fs.h> | 34 | #include <linux/fs.h> |
35 | #include <linux/poll.h> | 35 | #include <linux/poll.h> |
36 | #include <linux/module.h> | 36 | #include <linux/module.h> |
37 | #include <linux/mutex.h> | ||
37 | #include <linux/init.h> | 38 | #include <linux/init.h> |
38 | #include <linux/interrupt.h> | 39 | #include <linux/interrupt.h> |
39 | #include <linux/vmalloc.h> | 40 | #include <linux/vmalloc.h> |
@@ -2541,11 +2542,18 @@ static int raw1394_read_cycle_timer(struct file_info *fi, void __user * uaddr) | |||
2541 | static int raw1394_mmap(struct file *file, struct vm_area_struct *vma) | 2542 | static int raw1394_mmap(struct file *file, struct vm_area_struct *vma) |
2542 | { | 2543 | { |
2543 | struct file_info *fi = file->private_data; | 2544 | struct file_info *fi = file->private_data; |
2545 | int ret; | ||
2546 | |||
2547 | mutex_lock(&fi->state_mutex); | ||
2544 | 2548 | ||
2545 | if (fi->iso_state == RAW1394_ISO_INACTIVE) | 2549 | if (fi->iso_state == RAW1394_ISO_INACTIVE) |
2546 | return -EINVAL; | 2550 | ret = -EINVAL; |
2551 | else | ||
2552 | ret = dma_region_mmap(&fi->iso_handle->data_buf, file, vma); | ||
2553 | |||
2554 | mutex_unlock(&fi->state_mutex); | ||
2547 | 2555 | ||
2548 | return dma_region_mmap(&fi->iso_handle->data_buf, file, vma); | 2556 | return ret; |
2549 | } | 2557 | } |
2550 | 2558 | ||
2551 | /* ioctl is only used for rawiso operations */ | 2559 | /* ioctl is only used for rawiso operations */ |
@@ -2659,10 +2667,12 @@ static long do_raw1394_ioctl(struct file *file, unsigned int cmd, | |||
2659 | static long raw1394_ioctl(struct file *file, unsigned int cmd, | 2667 | static long raw1394_ioctl(struct file *file, unsigned int cmd, |
2660 | unsigned long arg) | 2668 | unsigned long arg) |
2661 | { | 2669 | { |
2670 | struct file_info *fi = file->private_data; | ||
2662 | long ret; | 2671 | long ret; |
2663 | lock_kernel(); | 2672 | |
2673 | mutex_lock(&fi->state_mutex); | ||
2664 | ret = do_raw1394_ioctl(file, cmd, arg); | 2674 | ret = do_raw1394_ioctl(file, cmd, arg); |
2665 | unlock_kernel(); | 2675 | mutex_unlock(&fi->state_mutex); |
2666 | return ret; | 2676 | return ret; |
2667 | } | 2677 | } |
2668 | 2678 | ||
@@ -2724,7 +2734,7 @@ static long raw1394_compat_ioctl(struct file *file, | |||
2724 | void __user *argp = (void __user *)arg; | 2734 | void __user *argp = (void __user *)arg; |
2725 | long err; | 2735 | long err; |
2726 | 2736 | ||
2727 | lock_kernel(); | 2737 | mutex_lock(&fi->state_mutex); |
2728 | switch (cmd) { | 2738 | switch (cmd) { |
2729 | /* These requests have same format as long as 'int' has same size. */ | 2739 | /* These requests have same format as long as 'int' has same size. */ |
2730 | case RAW1394_IOC_ISO_RECV_INIT: | 2740 | case RAW1394_IOC_ISO_RECV_INIT: |
@@ -2757,7 +2767,7 @@ static long raw1394_compat_ioctl(struct file *file, | |||
2757 | err = -EINVAL; | 2767 | err = -EINVAL; |
2758 | break; | 2768 | break; |
2759 | } | 2769 | } |
2760 | unlock_kernel(); | 2770 | mutex_unlock(&fi->state_mutex); |
2761 | 2771 | ||
2762 | return err; | 2772 | return err; |
2763 | } | 2773 | } |
@@ -2791,6 +2801,7 @@ static int raw1394_open(struct inode *inode, struct file *file) | |||
2791 | fi->notification = (u8) RAW1394_NOTIFY_ON; /* busreset notification */ | 2801 | fi->notification = (u8) RAW1394_NOTIFY_ON; /* busreset notification */ |
2792 | 2802 | ||
2793 | INIT_LIST_HEAD(&fi->list); | 2803 | INIT_LIST_HEAD(&fi->list); |
2804 | mutex_init(&fi->state_mutex); | ||
2794 | fi->state = opened; | 2805 | fi->state = opened; |
2795 | INIT_LIST_HEAD(&fi->req_pending); | 2806 | INIT_LIST_HEAD(&fi->req_pending); |
2796 | INIT_LIST_HEAD(&fi->req_complete); | 2807 | INIT_LIST_HEAD(&fi->req_complete); |