Messages in this thread | | | From | Arnd Bergmann <> | Date | Thu, 8 Oct 2020 21:35:32 +0200 | Subject | Re: [PATCH] char: ppdev: check if ioctl argument is present and valid |
| |
On Thu, Oct 8, 2020 at 8:27 PM Harshal Chaudhari <harshalchau04@gmail.com> wrote: > > Checking the argument passed to the ioctl is valid > or not. if not then return -EINVAL. > > Signed-off-by: Harshal Chaudhari <harshalchau04@gmail.com> > --- > drivers/char/ppdev.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c > index 38b46c7d1737..001392980202 100644 > --- a/drivers/char/ppdev.c > +++ b/drivers/char/ppdev.c > @@ -354,7 +354,7 @@ static int pp_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg) > unsigned int minor = iminor(file_inode(file)); > struct pp_struct *pp = file->private_data; > struct parport *port; > - void __user *argp = (void __user *)arg; > + void __user *argp = NULL; > struct ieee1284_info *info; > unsigned char reg; > unsigned char mask;
Assigning to NULL serves no purpose here.
> @@ -364,6 +364,16 @@ static int pp_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg) > struct timespec64 ts; > int ret; > > + if (_IOC_TYPE(cmd) != PP_IOCTL) > + return -ENOTTY;
This looks correct but is normally done as a "default" case
> + /* check if ioctl argument is present and valid */ > + if (_IOC_DIR(cmd) != _IOC_NONE) { > + argp = (void __user *)arg; > + if (!argp) > + return -EINVAL; > + }
This is a change in behavior, it changes the return code from the correct -EFAULT to an unusual -EINVAL for the special case that the pointer is NULL compared to other invalid pointers.
Arnd
| |