| Message ID | 1450346332-31830-3-git-send-email-bamvor.zhangjian@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On Thursday 17 December 2015 17:58:52 Bamvor Jian Zhang wrote: > The arg of ioctl in ppdev is the pointer of integer except the > timeval in PPSETTIME, PPGETTIME. Different size of timeval > is already supported by the previous patches. So, it is safe > to add compat support. > > Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org> > Reviewed-by: Arnd Bergmann <arnd@arndb.de> (I think I replied with the reviewed-by tag before to this patch) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Fri, Dec 18, 2015 at 12:12:05AM +0100, Arnd Bergmann wrote: > On Thursday 17 December 2015 17:58:52 Bamvor Jian Zhang wrote: > > The arg of ioctl in ppdev is the pointer of integer except the > > timeval in PPSETTIME, PPGETTIME. Different size of timeval > > is already supported by the previous patches. So, it is safe > > to add compat support. > > > > Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org> > > > > Reviewed-by: Arnd Bergmann <arnd@arndb.de> > > (I think I replied with the reviewed-by tag before to this patch) I was testing this series today. And it is breaking my userspace code. I am attaching my userspace code for you to check. Its very simple userspace code: 1: open 2: ioctl to claim 3: ioctl - PPGETTIME 4: ioctl - PPSETTIME 5: ioctl - PPGETTIME 6: ioctl - release 7: close Without this series it works as expected. With this series applied, the userspace code prints the error message: PPNEGOT: Bad address I traced it with strace and: ioctl(3, PPGETTIME, 0xbfe91508) = -1 EFAULT (Bad address) regards sudip
Hi, Sudip On 12/30/2015 07:16 PM, Sudip Mukherjee wrote: > On Fri, Dec 18, 2015 at 12:12:05AM +0100, Arnd Bergmann wrote: >> On Thursday 17 December 2015 17:58:52 Bamvor Jian Zhang wrote: >>> The arg of ioctl in ppdev is the pointer of integer except the >>> timeval in PPSETTIME, PPGETTIME. Different size of timeval >>> is already supported by the previous patches. So, it is safe >>> to add compat support. >>> >>> Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org> >>> >> >> Reviewed-by: Arnd Bergmann <arnd@arndb.de> >> >> (I think I replied with the reviewed-by tag before to this patch) > > I was testing this series today. And it is breaking my userspace code. I > am attaching my userspace code for you to check. Its very simple > userspace code: > 1: open > 2: ioctl to claim > 3: ioctl - PPGETTIME > 4: ioctl - PPSETTIME > 5: ioctl - PPGETTIME > 6: ioctl - release > 7: close > > Without this series it works as expected. > > With this series applied, the userspace code prints the error message: > PPNEGOT: Bad address > > I traced it with strace and: > ioctl(3, PPGETTIME, 0xbfe91508) = -1 EFAULT (Bad address) Thanks for your testing. It seems that I misuse the parameters. Could you please apply the following patch and try it again? There is no parport in my computer, Thanks. Regards Bamvor > > regards > sudip > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c index 31bc7b7..9e98d01 100644 --- a/drivers/char/ppdev.c +++ b/drivers/char/ppdev.c @@ -636,7 +636,7 @@ static int pp_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg) if ((time32[0] < 0) || (time32[1] < 0)) return -EINVAL; - if (copy_to_user(time32, argp, sizeof(time32))) + if (copy_to_user(argp, time32, sizeof(time32))) return -EFAULT; return 0; @@ -648,7 +648,7 @@ static int pp_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg) if ((time64[0] < 0) || (time64[1] < 0)) return -EINVAL; - if (copy_to_user(time64, argp, sizeof(time64))) + if (copy_to_user(argp, time64, sizeof(time64))) return -EFAULT; return 0;
diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c index 19a4d6e..31bc7b7 100644 --- a/drivers/char/ppdev.c +++ b/drivers/char/ppdev.c @@ -69,6 +69,7 @@ #include <linux/ppdev.h> #include <linux/mutex.h> #include <linux/uaccess.h> +#include <linux/compat.h> #define PP_VERSION "ppdev: user-space parallel port driver" #define CHRDEV "ppdev" @@ -670,6 +671,14 @@ static long pp_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return ret; } +#ifdef CONFIG_COMPAT +static long pp_compat_ioctl(struct file *file, unsigned int cmd, + unsigned long arg) +{ + return pp_ioctl(file, cmd, (unsigned long)compat_ptr(arg)); +} +#endif + static int pp_open (struct inode * inode, struct file * file) { unsigned int minor = iminor(inode); @@ -779,6 +788,9 @@ static const struct file_operations pp_fops = { .write = pp_write, .poll = pp_poll, .unlocked_ioctl = pp_ioctl, +#ifdef CONFIG_COMPAT + .compat_ioctl = pp_compat_ioctl, +#endif .open = pp_open, .release = pp_release, };
The arg of ioctl in ppdev is the pointer of integer except the timeval in PPSETTIME, PPGETTIME. Different size of timeval is already supported by the previous patches. So, it is safe to add compat support. Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org> --- drivers/char/ppdev.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/