2010-07-21 00:58:25

by Hartley Sweeten

[permalink] [raw]
Subject: [PATCH] Staging: dt3155: Remove copy_to_user for ioctl's DT3155_{STOP|START}

The ioctl's DT3155_STOP and DT3155_START are defined with the macro
_IO indicating that they have no parameters. They should not be using
copy_to_user to return data to user space.

Signed-off-by: H Hartley Sweeten <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Scott Smedley <[email protected]>

---

diff --git a/drivers/staging/dt3155/dt3155_drv.c b/drivers/staging/dt3155/dt3155_drv.c
index fed7e62..5eb1bcd 100644
--- a/drivers/staging/dt3155/dt3155_drv.c
+++ b/drivers/staging/dt3155/dt3155_drv.c
@@ -570,8 +570,6 @@ static int dt3155_ioctl(struct inode *inode,
return 0;

quick_stop(minor);
- if (copy_to_user(up, dts, sizeof(*dts)))
- return -EFAULT;
return 0;
}
case DT3155_START:
@@ -593,8 +591,6 @@ static int dt3155_ioctl(struct inode *inode,
}

dt3155_init_isr(minor);
- if (copy_to_user(up, dts, sizeof(*dts)))
- return -EFAULT;
return 0;
}
default:


2010-07-21 07:57:39

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH] Staging: dt3155: Remove copy_to_user for ioctl's DT3155_{STOP|START}

On 07/21/2010 02:57 AM, H Hartley Sweeten wrote:
> The ioctl's DT3155_STOP and DT3155_START are defined with the macro
> _IO indicating that they have no parameters. They should not be using
> copy_to_user to return data to user space.

This is not a reason for removing them. You should check what real users
do/expect and base your reasoning on the top of that.

So could you investigate that?

regards,
--
js

2010-07-22 18:00:49

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] Staging: dt3155: Remove copy_to_user for ioctl's DT3155_{STOP|START}

On Wed, Jul 21, 2010 at 09:57:16AM +0200, Jiri Slaby wrote:
> On 07/21/2010 02:57 AM, H Hartley Sweeten wrote:
> > The ioctl's DT3155_STOP and DT3155_START are defined with the macro
> > _IO indicating that they have no parameters. They should not be using
> > copy_to_user to return data to user space.
>
> This is not a reason for removing them. You should check what real users
> do/expect and base your reasoning on the top of that.
>
> So could you investigate that?

I agree, we can't remove this, it changes the functionality of the
kernel in a way that userspace might not be expecting.

thanks,

greg k-h