2009-03-24 21:15:40

by Stoyan Gaydarov

Subject: [PATCH 07/13] [usb] changed ioctls to unlocked

From: Stoyan Gaydarov <[email protected]>

Signed-off-by: Stoyan Gaydarov <[email protected]>
---
drivers/usb/mon/mon_bin.c | 53 ++++++++++++++++++++++++++++++++------------
1 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/mon/mon_bin.c b/drivers/usb/mon/mon_bin.c
index 4cf27c7..7789b7b 100644
--- a/drivers/usb/mon/mon_bin.c
+++ b/drivers/usb/mon/mon_bin.c
@@ -836,9 +836,10 @@ static int mon_bin_queued(struct mon_reader_bin *rp)

/*
*/
-static int mon_bin_ioctl(struct inode *inode, struct file *file,
- unsigned int cmd, unsigned long arg)
+static long mon_bin_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
{
+ lock_kernel();
struct mon_reader_bin *rp = file->private_data;
// struct mon_bus* mbus = rp->r.m_bus;
int ret = 0;
@@ -874,8 +875,10 @@ static int mon_bin_ioctl(struct inode *inode, struct file *file,
int size;
struct mon_pgmap *vec;

- if (arg < BUFF_MIN || arg > BUFF_MAX)
+ if (arg < BUFF_MIN || arg > BUFF_MAX) {
+ unlock_kernel();
return -EINVAL;
+ }

size = CHUNK_ALIGN(arg);
if ((vec = kzalloc(sizeof(struct mon_pgmap) * (size/CHUNK_SIZE),
@@ -912,11 +915,15 @@ static int mon_bin_ioctl(struct inode *inode, struct file *file,
struct mon_bin_get getb;

if (copy_from_user(&getb, (void __user *)arg,
- sizeof(struct mon_bin_get)))
+ sizeof(struct mon_bin_get))) {
+ unlock_kernel();
return -EFAULT;
+ }

- if (getb.alloc > 0x10000000) /* Want to cast to u32 */
+ if (getb.alloc > 0x10000000) { /* Want to cast to u32 */
+ unlock_kernel();
return -EINVAL;
+ }
ret = mon_bin_get_event(file, rp,
getb.hdr, getb.data, (unsigned int)getb.alloc);
}
@@ -929,21 +936,31 @@ static int mon_bin_ioctl(struct inode *inode, struct file *file,

uptr = (struct mon_bin_mfetch __user *)arg;

- if (copy_from_user(&mfetch, uptr, sizeof(mfetch)))
+ if (copy_from_user(&mfetch, uptr, sizeof(mfetch))) {
+ unlock_kernel();
return -EFAULT;
+ }

if (mfetch.nflush) {
ret = mon_bin_flush(rp, mfetch.nflush);
- if (ret < 0)
+ if (ret < 0) {
+ unlock_kernel();
return ret;
- if (put_user(ret, &uptr->nflush))
+ }
+ if (put_user(ret, &uptr->nflush)) {
+ unlock_kernel();
return -EFAULT;
+ }
}
ret = mon_bin_fetch(file, rp, mfetch.offvec, mfetch.nfetch);
- if (ret < 0)
+ if (ret < 0) {
+ unlock_kernel();
return ret;
- if (put_user(ret, &uptr->nfetch))
+ }
+ if (put_user(ret, &uptr->nfetch)) {
+ unlock_kernel();
return -EFAULT;
+ }
ret = 0;
}
break;
@@ -960,18 +977,24 @@ static int mon_bin_ioctl(struct inode *inode, struct file *file,
nevents = mon_bin_queued(rp);

sp = (struct mon_bin_stats __user *)arg;
- if (put_user(rp->cnt_lost, &sp->dropped))
+ if (put_user(rp->cnt_lost, &sp->dropped)) {
+ unlock_kernel();
return -EFAULT;
- if (put_user(nevents, &sp->queued))
+ }
+ if (put_user(nevents, &sp->queued)) {
+ unlock_kernel();
return -EFAULT;
+ }

}
break;

default:
+ unlock_kernel();
return -ENOTTY;
}

+ unlock_kernel();
return ret;
}

@@ -1026,14 +1049,14 @@ static long mon_bin_compat_ioctl(struct file *file,
return 0;

case MON_IOCG_STATS:
- return mon_bin_ioctl(NULL, file, cmd,
+ return mon_bin_ioctl(file, cmd,
(unsigned long) compat_ptr(arg));

case MON_IOCQ_URB_LEN:
case MON_IOCQ_RING_SIZE:
case MON_IOCT_RING_SIZE:
case MON_IOCH_MFLUSH:
- return mon_bin_ioctl(NULL, file, cmd, arg);
+ return mon_bin_ioctl(file, cmd, arg);

default:
;
@@ -1117,7 +1140,7 @@ static const struct file_operations mon_fops_binary = {
.read = mon_bin_read,
/* .write = mon_text_write, */
.poll = mon_bin_poll,
- .ioctl = mon_bin_ioctl,
+ .unlocked_ioctl = mon_bin_ioctl,
#ifdef CONFIG_COMPAT
.compat_ioctl = mon_bin_compat_ioctl,
#endif
--
1.6.2

2009-03-24 22:15:13

by Greg KH

Subject: Re: [PATCH 07/13] [usb] changed ioctls to unlocked

On Tue, Mar 24, 2009 at 04:12:42PM -0500, [email protected] wrote:
> From: Stoyan Gaydarov <[email protected]>
>
> Signed-off-by: Stoyan Gaydarov <[email protected]>
> ---
> drivers/usb/mon/mon_bin.c | 53 ++++++++++++++++++++++++++++++++------------
> 1 files changed, 38 insertions(+), 15 deletions(-)

NAK, you're just pushing lock_kernel() down for no good reason.

the code should be fixed to not need the lock at all.

thanks,

greg k-h

2009-03-24 22:20:39

by Alan

Subject: Re: [PATCH 07/13] [usb] changed ioctls to unlocked

On Tue, 24 Mar 2009 15:09:53 -0700
Greg KH <[email protected]> wrote:

> On Tue, Mar 24, 2009 at 04:12:42PM -0500, [email protected] wrote:
> > From: Stoyan Gaydarov <[email protected]>
> >
> > Signed-off-by: Stoyan Gaydarov <[email protected]>
> > ---
> > drivers/usb/mon/mon_bin.c | 53 ++++++++++++++++++++++++++++++++------------
> > 1 files changed, 38 insertions(+), 15 deletions(-)
>
> NAK, you're just pushing lock_kernel() down for no good reason.

Actually the underlying reason is to get rid of the old ->ioctl hook
entirely so it makes sense to do this but yes for simple drivers they
just want fixing.