[Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read

[Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read

[jaskaransingh7654321]

Hi,

Following is my analysis of syzbot report:
https://syzkaller.appspot.com/bug?id=688d05e810900459b03c82fb48dc609edbbf7df9

The bug pertains to a divide error. The report's call trace is as follows:

 usbtmc_ioctl_generic_read drivers/usb/class/usbtmc.c:1029 [inline]
 usbtmc_ioctl+0x27d/0x2ab0 drivers/usb/class/usbtmc.c:2089
 vfs_ioctl fs/ioctl.c:46 [inline]
 file_ioctl fs/ioctl.c:509 [inline]
 do_vfs_ioctl+0xd2d/0x1330 fs/ioctl.c:696
 ksys_ioctl+0x9b/0xc0 fs/ioctl.c:713
 __do_sys_ioctl fs/ioctl.c:720 [inline]
 __se_sys_ioctl fs/ioctl.c:718 [inline]
 __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:718
 do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

The file:line where the error occurs is this:
	drivers/usb/class/usbtmc.c:816

Line 816 of that file is as follows:
		if ((max_transfer_size % data->wMaxPacketSize) == 0)

Running a 'git blame' on the file in question reveals that the last
modification to line 816 was commit bb99794a4792068cb4bfd40e99e0f9d8fe7872fa.
The commit adds the USBTMC_IOCTL_READ call for the driver, and thus the
usbtmc_generic_read function.

I was not able to reproduce the error using my native machine and the
same gcc version used by syzkaller (gcc 9.0.0 20181231). However, the
error probably occurs because data->wMaxPacketSize is zero.

An intuitive fix for this seems to be:

-		if ((max_transfer_size % data->wMaxPacketSize) == 0)
+		if (data->wMaxPacketSize &&
+		    (max_transfer_size % data->wMaxPacketSize) == 0)

Or perhaps introduce a guard condition above line 816 to raise an error.

Cheers,
Jaskaran.

[Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read

[Jaskaran Singh]

Hi,

Following is my analysis of syzbot report:
https://syzkaller.appspot.com/bug?id=688d05e810900459b03c82fb48dc609edbbf7df9

The bug pertains to a divide error. The report's call trace is as follows:

 usbtmc_ioctl_generic_read drivers/usb/class/usbtmc.c:1029 [inline]
 usbtmc_ioctl+0x27d/0x2ab0 drivers/usb/class/usbtmc.c:2089
 vfs_ioctl fs/ioctl.c:46 [inline]
 file_ioctl fs/ioctl.c:509 [inline]
 do_vfs_ioctl+0xd2d/0x1330 fs/ioctl.c:696
 ksys_ioctl+0x9b/0xc0 fs/ioctl.c:713
 __do_sys_ioctl fs/ioctl.c:720 [inline]
 __se_sys_ioctl fs/ioctl.c:718 [inline]
 __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:718
 do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

The file:line where the error occurs is this:
	drivers/usb/class/usbtmc.c:816

Line 816 of that file is as follows:
		if ((max_transfer_size % data->wMaxPacketSize) == 0)

Running a 'git blame' on the file in question reveals that the last
modification to line 816 was commit bb99794a4792068cb4bfd40e99e0f9d8fe7872fa.
The commit adds the USBTMC_IOCTL_READ call for the driver, and thus the
usbtmc_generic_read function.

I was not able to reproduce the error using my native machine and the
same gcc version used by syzkaller (gcc 9.0.0 20181231). However, the
error probably occurs because data->wMaxPacketSize is zero.

An intuitive fix for this seems to be:

-		if ((max_transfer_size % data->wMaxPacketSize) == 0)
+		if (data->wMaxPacketSize &&
+		    (max_transfer_size % data->wMaxPacketSize) == 0)

Or perhaps introduce a guard condition above line 816 to raise an error.

Cheers,
Jaskaran.

[Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read

[gregkh]

On Fri, Oct 04, 2019 at 01:00:16AM +0530, Jaskaran Singh wrote:
> Hi,
> 
> Following is my analysis of syzbot report:
> https://syzkaller.appspot.com/bug?id=688d05e810900459b03c82fb48dc609edbbf7df9
> 
> The bug pertains to a divide error. The report's call trace is as follows:
> 
>  usbtmc_ioctl_generic_read drivers/usb/class/usbtmc.c:1029 [inline]
>  usbtmc_ioctl+0x27d/0x2ab0 drivers/usb/class/usbtmc.c:2089
>  vfs_ioctl fs/ioctl.c:46 [inline]
>  file_ioctl fs/ioctl.c:509 [inline]
>  do_vfs_ioctl+0xd2d/0x1330 fs/ioctl.c:696
>  ksys_ioctl+0x9b/0xc0 fs/ioctl.c:713
>  __do_sys_ioctl fs/ioctl.c:720 [inline]
>  __se_sys_ioctl fs/ioctl.c:718 [inline]
>  __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:718
>  do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> The file:line where the error occurs is this:
> 	drivers/usb/class/usbtmc.c:816
> 
> Line 816 of that file is as follows:
> 		if ((max_transfer_size % data->wMaxPacketSize) == 0)
> 
> Running a 'git blame' on the file in question reveals that the last
> modification to line 816 was commit bb99794a4792068cb4bfd40e99e0f9d8fe7872fa.
> The commit adds the USBTMC_IOCTL_READ call for the driver, and thus the
> usbtmc_generic_read function.
> 
> I was not able to reproduce the error using my native machine and the
> same gcc version used by syzkaller (gcc 9.0.0 20181231). However, the
> error probably occurs because data->wMaxPacketSize is zero.
> 
> An intuitive fix for this seems to be:
> 
> -		if ((max_transfer_size % data->wMaxPacketSize) == 0)
> +		if (data->wMaxPacketSize &&
> +		    (max_transfer_size % data->wMaxPacketSize) == 0)
> 
> Or perhaps introduce a guard condition above line 816 to raise an error.

Yes, if wMaxPacketSize is 0, then the code should just return an error.
Care to make up a patch for this and submit it to syzbot to see if it
solves the problem or not?

thanks,

greg k-h

[Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read

[Greg KH]

On Fri, Oct 04, 2019 at 01:00:16AM +0530, Jaskaran Singh wrote:
> Hi,
> 
> Following is my analysis of syzbot report:
> https://syzkaller.appspot.com/bug?id=688d05e810900459b03c82fb48dc609edbbf7df9
> 
> The bug pertains to a divide error. The report's call trace is as follows:
> 
>  usbtmc_ioctl_generic_read drivers/usb/class/usbtmc.c:1029 [inline]
>  usbtmc_ioctl+0x27d/0x2ab0 drivers/usb/class/usbtmc.c:2089
>  vfs_ioctl fs/ioctl.c:46 [inline]
>  file_ioctl fs/ioctl.c:509 [inline]
>  do_vfs_ioctl+0xd2d/0x1330 fs/ioctl.c:696
>  ksys_ioctl+0x9b/0xc0 fs/ioctl.c:713
>  __do_sys_ioctl fs/ioctl.c:720 [inline]
>  __se_sys_ioctl fs/ioctl.c:718 [inline]
>  __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:718
>  do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> The file:line where the error occurs is this:
> 	drivers/usb/class/usbtmc.c:816
> 
> Line 816 of that file is as follows:
> 		if ((max_transfer_size % data->wMaxPacketSize) == 0)
> 
> Running a 'git blame' on the file in question reveals that the last
> modification to line 816 was commit bb99794a4792068cb4bfd40e99e0f9d8fe7872fa.
> The commit adds the USBTMC_IOCTL_READ call for the driver, and thus the
> usbtmc_generic_read function.
> 
> I was not able to reproduce the error using my native machine and the
> same gcc version used by syzkaller (gcc 9.0.0 20181231). However, the
> error probably occurs because data->wMaxPacketSize is zero.
> 
> An intuitive fix for this seems to be:
> 
> -		if ((max_transfer_size % data->wMaxPacketSize) == 0)
> +		if (data->wMaxPacketSize &&
> +		    (max_transfer_size % data->wMaxPacketSize) == 0)
> 
> Or perhaps introduce a guard condition above line 816 to raise an error.

Yes, if wMaxPacketSize is 0, then the code should just return an error.
Care to make up a patch for this and submit it to syzbot to see if it
solves the problem or not?

thanks,

greg k-h

[Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read

[jaskaransingh7654321]

On Thu, 2019-10-03 at 21:56 +0200, Greg KH wrote:
> On Fri, Oct 04, 2019 at 01:00:16AM +0530, Jaskaran Singh wrote:
> > Hi,
> > 
> > Following is my analysis of syzbot report:
> > https://syzkaller.appspot.com/bug?id=688d05e810900459b03c82fb48dc609edbbf7df9
> > 
> > The bug pertains to a divide error. The report's call trace is as
> > follows:
> > 
> >  usbtmc_ioctl_generic_read drivers/usb/class/usbtmc.c:1029 [inline]
> >  usbtmc_ioctl+0x27d/0x2ab0 drivers/usb/class/usbtmc.c:2089
> >  vfs_ioctl fs/ioctl.c:46 [inline]
> >  file_ioctl fs/ioctl.c:509 [inline]
> >  do_vfs_ioctl+0xd2d/0x1330 fs/ioctl.c:696
> >  ksys_ioctl+0x9b/0xc0 fs/ioctl.c:713
> >  __do_sys_ioctl fs/ioctl.c:720 [inline]
> >  __se_sys_ioctl fs/ioctl.c:718 [inline]
> >  __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:718
> >  do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
> >  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > 
> > The file:line where the error occurs is this:
> > 	drivers/usb/class/usbtmc.c:816
> > 
> > Line 816 of that file is as follows:
> > 		if ((max_transfer_size % data->wMaxPacketSize) == 0)
> > 
> > Running a 'git blame' on the file in question reveals that the last
> > modification to line 816 was commit
> > bb99794a4792068cb4bfd40e99e0f9d8fe7872fa.
> > The commit adds the USBTMC_IOCTL_READ call for the driver, and thus
> > the
> > usbtmc_generic_read function.
> > 
> > I was not able to reproduce the error using my native machine and
> > the
> > same gcc version used by syzkaller (gcc 9.0.0 20181231). However,
> > the
> > error probably occurs because data->wMaxPacketSize is zero.
> > 
> > An intuitive fix for this seems to be:
> > 
> > -		if ((max_transfer_size % data->wMaxPacketSize) == 0)
> > +		if (data->wMaxPacketSize &&
> > +		    (max_transfer_size % data->wMaxPacketSize) == 0)
> > 
> > Or perhaps introduce a guard condition above line 816 to raise an
> > error.
> 
> Yes, if wMaxPacketSize is 0, then the code should just return an
> error.
> Care to make up a patch for this and submit it to syzbot to see if it
> solves the problem or not?
> 
> thanks,
> 
> greg k-h

Hi Greg,

>From the following thread:

https://groups.google.com/forum/#!searchin/syzkaller-bugs/divide$20error%7Csort:date/syzkaller-bugs/RYfg7J46e4c/t2amCUU7DgAJ

It looks like a patch has already been submitted and a crash does not
trigger with it. However, it might make sense to add more sanity
checking. Should I submit the patch?

Cheers,
Jaskaran.

[Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read

[Jaskaran Singh]

On Thu, 2019-10-03 at 21:56 +0200, Greg KH wrote:
> On Fri, Oct 04, 2019 at 01:00:16AM +0530, Jaskaran Singh wrote:
> > Hi,
> > 
> > Following is my analysis of syzbot report:
> > https://syzkaller.appspot.com/bug?id=688d05e810900459b03c82fb48dc609edbbf7df9
> > 
> > The bug pertains to a divide error. The report's call trace is as
> > follows:
> > 
> >  usbtmc_ioctl_generic_read drivers/usb/class/usbtmc.c:1029 [inline]
> >  usbtmc_ioctl+0x27d/0x2ab0 drivers/usb/class/usbtmc.c:2089
> >  vfs_ioctl fs/ioctl.c:46 [inline]
> >  file_ioctl fs/ioctl.c:509 [inline]
> >  do_vfs_ioctl+0xd2d/0x1330 fs/ioctl.c:696
> >  ksys_ioctl+0x9b/0xc0 fs/ioctl.c:713
> >  __do_sys_ioctl fs/ioctl.c:720 [inline]
> >  __se_sys_ioctl fs/ioctl.c:718 [inline]
> >  __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:718
> >  do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
> >  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > 
> > The file:line where the error occurs is this:
> > 	drivers/usb/class/usbtmc.c:816
> > 
> > Line 816 of that file is as follows:
> > 		if ((max_transfer_size % data->wMaxPacketSize) == 0)
> > 
> > Running a 'git blame' on the file in question reveals that the last
> > modification to line 816 was commit
> > bb99794a4792068cb4bfd40e99e0f9d8fe7872fa.
> > The commit adds the USBTMC_IOCTL_READ call for the driver, and thus
> > the
> > usbtmc_generic_read function.
> > 
> > I was not able to reproduce the error using my native machine and
> > the
> > same gcc version used by syzkaller (gcc 9.0.0 20181231). However,
> > the
> > error probably occurs because data->wMaxPacketSize is zero.
> > 
> > An intuitive fix for this seems to be:
> > 
> > -		if ((max_transfer_size % data->wMaxPacketSize) == 0)
> > +		if (data->wMaxPacketSize &&
> > +		    (max_transfer_size % data->wMaxPacketSize) == 0)
> > 
> > Or perhaps introduce a guard condition above line 816 to raise an
> > error.
> 
> Yes, if wMaxPacketSize is 0, then the code should just return an
> error.
> Care to make up a patch for this and submit it to syzbot to see if it
> solves the problem or not?
> 
> thanks,
> 
> greg k-h

Hi Greg,

>From the following thread:

https://groups.google.com/forum/#!searchin/syzkaller-bugs/divide$20error%7Csort:date/syzkaller-bugs/RYfg7J46e4c/t2amCUU7DgAJ

It looks like a patch has already been submitted and a crash does not
trigger with it. However, it might make sense to add more sanity
checking. Should I submit the patch?

Cheers,
Jaskaran.

[Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read

[gregkh]

On Fri, Oct 04, 2019 at 12:28:32PM +0530, Jaskaran Singh wrote:
> On Thu, 2019-10-03 at 21:56 +0200, Greg KH wrote:
> > On Fri, Oct 04, 2019 at 01:00:16AM +0530, Jaskaran Singh wrote:
> > > Hi,
> > > 
> > > Following is my analysis of syzbot report:
> > > https://syzkaller.appspot.com/bug?id=688d05e810900459b03c82fb48dc609edbbf7df9
> > > 
> > > The bug pertains to a divide error. The report's call trace is as
> > > follows:
> > > 
> > >  usbtmc_ioctl_generic_read drivers/usb/class/usbtmc.c:1029 [inline]
> > >  usbtmc_ioctl+0x27d/0x2ab0 drivers/usb/class/usbtmc.c:2089
> > >  vfs_ioctl fs/ioctl.c:46 [inline]
> > >  file_ioctl fs/ioctl.c:509 [inline]
> > >  do_vfs_ioctl+0xd2d/0x1330 fs/ioctl.c:696
> > >  ksys_ioctl+0x9b/0xc0 fs/ioctl.c:713
> > >  __do_sys_ioctl fs/ioctl.c:720 [inline]
> > >  __se_sys_ioctl fs/ioctl.c:718 [inline]
> > >  __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:718
> > >  do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
> > >  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > > 
> > > The file:line where the error occurs is this:
> > > 	drivers/usb/class/usbtmc.c:816
> > > 
> > > Line 816 of that file is as follows:
> > > 		if ((max_transfer_size % data->wMaxPacketSize) == 0)
> > > 
> > > Running a 'git blame' on the file in question reveals that the last
> > > modification to line 816 was commit
> > > bb99794a4792068cb4bfd40e99e0f9d8fe7872fa.
> > > The commit adds the USBTMC_IOCTL_READ call for the driver, and thus
> > > the
> > > usbtmc_generic_read function.
> > > 
> > > I was not able to reproduce the error using my native machine and
> > > the
> > > same gcc version used by syzkaller (gcc 9.0.0 20181231). However,
> > > the
> > > error probably occurs because data->wMaxPacketSize is zero.
> > > 
> > > An intuitive fix for this seems to be:
> > > 
> > > -		if ((max_transfer_size % data->wMaxPacketSize) == 0)
> > > +		if (data->wMaxPacketSize &&
> > > +		    (max_transfer_size % data->wMaxPacketSize) == 0)
> > > 
> > > Or perhaps introduce a guard condition above line 816 to raise an
> > > error.
> > 
> > Yes, if wMaxPacketSize is 0, then the code should just return an
> > error.
> > Care to make up a patch for this and submit it to syzbot to see if it
> > solves the problem or not?
> > 
> > thanks,
> > 
> > greg k-h
> 
> Hi Greg,
> 
> >From the following thread:
> 
> https://groups.google.com/forum/#!searchin/syzkaller-bugs/divide$20error%7Csort:date/syzkaller-bugs/RYfg7J46e4c/t2amCUU7DgAJ
> 
> It looks like a patch has already been submitted and a crash does not
> trigger with it. However, it might make sense to add more sanity
> checking. Should I submit the patch?

What more error checking can you add that is not already present with
the patch that was submitted and accepted already upstream?

And why look at a report that is already fixed?  Or is this one not
properly closed as fixed?  If not, can you please mark it as fixed?

thanks,

greg k-h

[Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read

[Greg KH]

On Fri, Oct 04, 2019 at 12:28:32PM +0530, Jaskaran Singh wrote:
> On Thu, 2019-10-03 at 21:56 +0200, Greg KH wrote:
> > On Fri, Oct 04, 2019 at 01:00:16AM +0530, Jaskaran Singh wrote:
> > > Hi,
> > > 
> > > Following is my analysis of syzbot report:
> > > https://syzkaller.appspot.com/bug?id=688d05e810900459b03c82fb48dc609edbbf7df9
> > > 
> > > The bug pertains to a divide error. The report's call trace is as
> > > follows:
> > > 
> > >  usbtmc_ioctl_generic_read drivers/usb/class/usbtmc.c:1029 [inline]
> > >  usbtmc_ioctl+0x27d/0x2ab0 drivers/usb/class/usbtmc.c:2089
> > >  vfs_ioctl fs/ioctl.c:46 [inline]
> > >  file_ioctl fs/ioctl.c:509 [inline]
> > >  do_vfs_ioctl+0xd2d/0x1330 fs/ioctl.c:696
> > >  ksys_ioctl+0x9b/0xc0 fs/ioctl.c:713
> > >  __do_sys_ioctl fs/ioctl.c:720 [inline]
> > >  __se_sys_ioctl fs/ioctl.c:718 [inline]
> > >  __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:718
> > >  do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
> > >  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > > 
> > > The file:line where the error occurs is this:
> > > 	drivers/usb/class/usbtmc.c:816
> > > 
> > > Line 816 of that file is as follows:
> > > 		if ((max_transfer_size % data->wMaxPacketSize) == 0)
> > > 
> > > Running a 'git blame' on the file in question reveals that the last
> > > modification to line 816 was commit
> > > bb99794a4792068cb4bfd40e99e0f9d8fe7872fa.
> > > The commit adds the USBTMC_IOCTL_READ call for the driver, and thus
> > > the
> > > usbtmc_generic_read function.
> > > 
> > > I was not able to reproduce the error using my native machine and
> > > the
> > > same gcc version used by syzkaller (gcc 9.0.0 20181231). However,
> > > the
> > > error probably occurs because data->wMaxPacketSize is zero.
> > > 
> > > An intuitive fix for this seems to be:
> > > 
> > > -		if ((max_transfer_size % data->wMaxPacketSize) == 0)
> > > +		if (data->wMaxPacketSize &&
> > > +		    (max_transfer_size % data->wMaxPacketSize) == 0)
> > > 
> > > Or perhaps introduce a guard condition above line 816 to raise an
> > > error.
> > 
> > Yes, if wMaxPacketSize is 0, then the code should just return an
> > error.
> > Care to make up a patch for this and submit it to syzbot to see if it
> > solves the problem or not?
> > 
> > thanks,
> > 
> > greg k-h
> 
> Hi Greg,
> 
> >From the following thread:
> 
> https://groups.google.com/forum/#!searchin/syzkaller-bugs/divide$20error%7Csort:date/syzkaller-bugs/RYfg7J46e4c/t2amCUU7DgAJ
> 
> It looks like a patch has already been submitted and a crash does not
> trigger with it. However, it might make sense to add more sanity
> checking. Should I submit the patch?

What more error checking can you add that is not already present with
the patch that was submitted and accepted already upstream?

And why look at a report that is already fixed?  Or is this one not
properly closed as fixed?  If not, can you please mark it as fixed?

thanks,

greg k-h

[Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read

[jaskaransingh7654321]

On Fri, 2019-10-04 at 09:21 +0200, Greg KH wrote:
> On Fri, Oct 04, 2019 at 12:28:32PM +0530, Jaskaran Singh wrote:
> > On Thu, 2019-10-03 at 21:56 +0200, Greg KH wrote:
> > > On Fri, Oct 04, 2019 at 01:00:16AM +0530, Jaskaran Singh wrote:
> > > > Hi,
> > > > 
> > > > Following is my analysis of syzbot report:
> > > > https://syzkaller.appspot.com/bug?id=688d05e810900459b03c82fb48dc609edbbf7df9
> > > > 
> > > > The bug pertains to a divide error. The report's call trace is
> > > > as
> > > > follows:
> > > > 
> > > >  usbtmc_ioctl_generic_read drivers/usb/class/usbtmc.c:1029
> > > > [inline]
> > > >  usbtmc_ioctl+0x27d/0x2ab0 drivers/usb/class/usbtmc.c:2089
> > > >  vfs_ioctl fs/ioctl.c:46 [inline]
> > > >  file_ioctl fs/ioctl.c:509 [inline]
> > > >  do_vfs_ioctl+0xd2d/0x1330 fs/ioctl.c:696
> > > >  ksys_ioctl+0x9b/0xc0 fs/ioctl.c:713
> > > >  __do_sys_ioctl fs/ioctl.c:720 [inline]
> > > >  __se_sys_ioctl fs/ioctl.c:718 [inline]
> > > >  __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:718
> > > >  do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
> > > >  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > > > 
> > > > The file:line where the error occurs is this:
> > > > 	drivers/usb/class/usbtmc.c:816
> > > > 
> > > > Line 816 of that file is as follows:
> > > > 		if ((max_transfer_size % data->wMaxPacketSize)
> > > > == 0)
> > > > 
> > > > Running a 'git blame' on the file in question reveals that the
> > > > last
> > > > modification to line 816 was commit
> > > > bb99794a4792068cb4bfd40e99e0f9d8fe7872fa.
> > > > The commit adds the USBTMC_IOCTL_READ call for the driver, and
> > > > thus
> > > > the
> > > > usbtmc_generic_read function.
> > > > 
> > > > I was not able to reproduce the error using my native machine
> > > > and
> > > > the
> > > > same gcc version used by syzkaller (gcc 9.0.0 20181231).
> > > > However,
> > > > the
> > > > error probably occurs because data->wMaxPacketSize is zero.
> > > > 
> > > > An intuitive fix for this seems to be:
> > > > 
> > > > -		if ((max_transfer_size % data->wMaxPacketSize)
> > > > == 0)
> > > > +		if (data->wMaxPacketSize &&
> > > > +		    (max_transfer_size % data->wMaxPacketSize)
> > > > == 0)
> > > > 
> > > > Or perhaps introduce a guard condition above line 816 to raise
> > > > an
> > > > error.
> > > 
> > > Yes, if wMaxPacketSize is 0, then the code should just return an
> > > error.
> > > Care to make up a patch for this and submit it to syzbot to see
> > > if it
> > > solves the problem or not?
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > 
> > Hi Greg,
> > 
> > > From the following thread:
> > 
> > https://groups.google.com/forum/#!searchin/syzkaller-bugs/divide$20error%7Csort:date/syzkaller-bugs/RYfg7J46e4c/t2amCUU7DgAJ
> > 
> > It looks like a patch has already been submitted and a crash does
> > not
> > trigger with it. However, it might make sense to add more sanity
> > checking. Should I submit the patch?
> 
> What more error checking can you add that is not already present with
> the patch that was submitted and accepted already upstream?
> 
> And why look at a report that is already fixed?  Or is this one not
> properly closed as fixed?  If not, can you please mark it as fixed?

It wasn't closed or marked as fixed on the syzkaller portal. I'll be
more careful and check the mailing list as well next time.

Cheers,
Jaskaran.


> thanks,
> 
> greg k-h

[Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read

[Jaskaran Singh]

On Fri, 2019-10-04 at 09:21 +0200, Greg KH wrote:
> On Fri, Oct 04, 2019 at 12:28:32PM +0530, Jaskaran Singh wrote:
> > On Thu, 2019-10-03 at 21:56 +0200, Greg KH wrote:
> > > On Fri, Oct 04, 2019 at 01:00:16AM +0530, Jaskaran Singh wrote:
> > > > Hi,
> > > > 
> > > > Following is my analysis of syzbot report:
> > > > https://syzkaller.appspot.com/bug?id=688d05e810900459b03c82fb48dc609edbbf7df9
> > > > 
> > > > The bug pertains to a divide error. The report's call trace is
> > > > as
> > > > follows:
> > > > 
> > > >  usbtmc_ioctl_generic_read drivers/usb/class/usbtmc.c:1029
> > > > [inline]
> > > >  usbtmc_ioctl+0x27d/0x2ab0 drivers/usb/class/usbtmc.c:2089
> > > >  vfs_ioctl fs/ioctl.c:46 [inline]
> > > >  file_ioctl fs/ioctl.c:509 [inline]
> > > >  do_vfs_ioctl+0xd2d/0x1330 fs/ioctl.c:696
> > > >  ksys_ioctl+0x9b/0xc0 fs/ioctl.c:713
> > > >  __do_sys_ioctl fs/ioctl.c:720 [inline]
> > > >  __se_sys_ioctl fs/ioctl.c:718 [inline]
> > > >  __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:718
> > > >  do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
> > > >  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > > > 
> > > > The file:line where the error occurs is this:
> > > > 	drivers/usb/class/usbtmc.c:816
> > > > 
> > > > Line 816 of that file is as follows:
> > > > 		if ((max_transfer_size % data->wMaxPacketSize)
> > > > == 0)
> > > > 
> > > > Running a 'git blame' on the file in question reveals that the
> > > > last
> > > > modification to line 816 was commit
> > > > bb99794a4792068cb4bfd40e99e0f9d8fe7872fa.
> > > > The commit adds the USBTMC_IOCTL_READ call for the driver, and
> > > > thus
> > > > the
> > > > usbtmc_generic_read function.
> > > > 
> > > > I was not able to reproduce the error using my native machine
> > > > and
> > > > the
> > > > same gcc version used by syzkaller (gcc 9.0.0 20181231).
> > > > However,
> > > > the
> > > > error probably occurs because data->wMaxPacketSize is zero.
> > > > 
> > > > An intuitive fix for this seems to be:
> > > > 
> > > > -		if ((max_transfer_size % data->wMaxPacketSize)
> > > > == 0)
> > > > +		if (data->wMaxPacketSize &&
> > > > +		    (max_transfer_size % data->wMaxPacketSize)
> > > > == 0)
> > > > 
> > > > Or perhaps introduce a guard condition above line 816 to raise
> > > > an
> > > > error.
> > > 
> > > Yes, if wMaxPacketSize is 0, then the code should just return an
> > > error.
> > > Care to make up a patch for this and submit it to syzbot to see
> > > if it
> > > solves the problem or not?
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > 
> > Hi Greg,
> > 
> > > From the following thread:
> > 
> > https://groups.google.com/forum/#!searchin/syzkaller-bugs/divide$20error%7Csort:date/syzkaller-bugs/RYfg7J46e4c/t2amCUU7DgAJ
> > 
> > It looks like a patch has already been submitted and a crash does
> > not
> > trigger with it. However, it might make sense to add more sanity
> > checking. Should I submit the patch?
> 
> What more error checking can you add that is not already present with
> the patch that was submitted and accepted already upstream?
> 
> And why look at a report that is already fixed?  Or is this one not
> properly closed as fixed?  If not, can you please mark it as fixed?

It wasn't closed or marked as fixed on the syzkaller portal. I'll be
more careful and check the mailing list as well next time.

Cheers,
Jaskaran.


> thanks,
> 
> greg k-h