Skip to contentSkip to conversation

Staging: dt3155: Remove copy_to_user for ioctl's DT3155_{STOP|START}

PATCH In review H Hartley Sweeten 3 participants 3 messages Jul 21, 2010 – Jul 22, 2010 Staging

lore
Share & tools
lore

H Hartley Sweeten

author
#0 lore raw

PATCH 1/1 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]>

---

drivers/staging/dt3155/dt3155_drv.c +0 -4
index fed7e62..5eb1bcd 100644
@@ -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:
#1 lore raw
Quoted 3 lines · H Hartley Sweeten

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,

Signature · 1 line
js
#2 lore raw
Quoted 9 lines · Jiri Slaby
Quoted 3 lines · H Hartley Sweeten

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