[PATCH] usb: gadget: f_fs: remove unused values and add immediate returns

[PATCH] usb: gadget: f_fs: remove unused values and add immediate returns

[Karol Przybylski]

In case of faulty copy_from_user call inside ffs_epfile_ioctl, error code is
saved in a variable. However, this variable is later overwritten in every possible
path, which overshadows initial assignment.

This patch fixes it by returning the error code immediately and exiting the function.

Error discovered in coverity scan - CID 1583682

Signed-off-by: Karol Przybylski <karprzy7@gmail.com>
---
 drivers/usb/gadget/function/f_fs.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 2920f8000bbd..00f52c9bb716 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -1735,8 +1735,7 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code,
 		int fd;
 
 		if (copy_from_user(&fd, (void __user *)value, sizeof(fd))) {
-			ret = -EFAULT;
-			break;
+			return -EFAULT;
 		}
 
 		return ffs_dmabuf_attach(file, fd);
@@ -1746,8 +1745,7 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code,
 		int fd;
 
 		if (copy_from_user(&fd, (void __user *)value, sizeof(fd))) {
-			ret = -EFAULT;
-			break;
+			return -EFAULT;
 		}
 
 		return ffs_dmabuf_detach(file, fd);
@@ -1757,8 +1755,7 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code,
 		struct usb_ffs_dmabuf_transfer_req req;
 
 		if (copy_from_user(&req, (void __user *)value, sizeof(req))) {
-			ret = -EFAULT;
-			break;
+			return -EFAULT;
 		}
 
 		return ffs_dmabuf_transfer(file, &req);
-- 
2.34.1

Re: [PATCH] usb: gadget: f_fs: remove unused values and add immediate returns

[Greg KH]

On Wed, Nov 06, 2024 at 07:30:32PM +0100, Karol Przybylski wrote:
> In case of faulty copy_from_user call inside ffs_epfile_ioctl, error code is
> saved in a variable. However, this variable is later overwritten in every possible
> path, which overshadows initial assignment.
> 
> This patch fixes it by returning the error code immediately and exiting the function.
> 
> Error discovered in coverity scan - CID 1583682
> 
> Signed-off-by: Karol Przybylski <karprzy7@gmail.com>
> ---
>  drivers/usb/gadget/function/f_fs.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)

What commit id does this fix?

How was this change tested?

thanks,

greg k-h

Re: [PATCH] usb: gadget: f_fs: remove unused values and add immediate returns

[Karol P]

On Thu, 7 Nov 2024 at 06:15, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Nov 06, 2024 at 07:30:32PM +0100, Karol Przybylski wrote:
> > In case of faulty copy_from_user call inside ffs_epfile_ioctl, error code is
> > saved in a variable. However, this variable is later overwritten in every possible
> > path, which overshadows initial assignment.
> >
> > This patch fixes it by returning the error code immediately and exiting the function.
> >
> > Error discovered in coverity scan - CID 1583682
> >
> > Signed-off-by: Karol Przybylski <karprzy7@gmail.com>
> > ---
> >  drivers/usb/gadget/function/f_fs.c | 9 +++------
> >  1 file changed, 3 insertions(+), 6 deletions(-)
>
> What commit id does this fix?

7b07a2a7ca02a, usb: gadget: functionfs: Add DMABUF import interface

>
> How was this change tested?

I compiled the kernel and ran it on my machine.
Are there other ways to reliably test such change?

>
> thanks,
>
> greg k-h

Re: [PATCH] usb: gadget: f_fs: remove unused values and add immediate returns

[Greg KH]

On Sun, Dec 15, 2024 at 12:41:03PM +0100, Karol P wrote:
> On Thu, 7 Nov 2024 at 06:15, Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Nov 06, 2024 at 07:30:32PM +0100, Karol Przybylski wrote:
> > > In case of faulty copy_from_user call inside ffs_epfile_ioctl, error code is
> > > saved in a variable. However, this variable is later overwritten in every possible
> > > path, which overshadows initial assignment.
> > >
> > > This patch fixes it by returning the error code immediately and exiting the function.
> > >
> > > Error discovered in coverity scan - CID 1583682
> > >
> > > Signed-off-by: Karol Przybylski <karprzy7@gmail.com>
> > > ---
> > >  drivers/usb/gadget/function/f_fs.c | 9 +++------
> > >  1 file changed, 3 insertions(+), 6 deletions(-)
> >
> > What commit id does this fix?
> 
> 7b07a2a7ca02a, usb: gadget: functionfs: Add DMABUF import interface

Then why not use the Fixes: tag?

> > How was this change tested?
> 
> I compiled the kernel and ran it on my machine.

Did you exercise this codepath?

> Are there other ways to reliably test such change?

Exercise the codepath by using the module and passing in a value to test
your change.

thanks,

greg k-h