* [Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read @ 2019-10-03 19:30 ` Jaskaran Singh 0 siblings, 0 replies; 10+ messages in thread From: jaskaransingh7654321 @ 2019-10-03 19:30 UTC (permalink / raw) 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read @ 2019-10-03 19:30 ` Jaskaran Singh 0 siblings, 0 replies; 10+ messages in thread From: Jaskaran Singh @ 2019-10-03 19:30 UTC (permalink / raw) 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read @ 2019-10-03 19:56 ` Greg KH 0 siblings, 0 replies; 10+ messages in thread From: gregkh @ 2019-10-03 19:56 UTC (permalink / raw) 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read @ 2019-10-03 19:56 ` Greg KH 0 siblings, 0 replies; 10+ messages in thread From: Greg KH @ 2019-10-03 19:56 UTC (permalink / raw) 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read @ 2019-10-04 6:58 ` Jaskaran Singh 0 siblings, 0 replies; 10+ messages in thread From: jaskaransingh7654321 @ 2019-10-04 6:58 UTC (permalink / raw) 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read @ 2019-10-04 6:58 ` Jaskaran Singh 0 siblings, 0 replies; 10+ messages in thread From: Jaskaran Singh @ 2019-10-04 6:58 UTC (permalink / raw) 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read @ 2019-10-04 7:21 ` Greg KH 0 siblings, 0 replies; 10+ messages in thread From: gregkh @ 2019-10-04 7:21 UTC (permalink / raw) 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read @ 2019-10-04 7:21 ` Greg KH 0 siblings, 0 replies; 10+ messages in thread From: Greg KH @ 2019-10-04 7:21 UTC (permalink / raw) 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read @ 2019-10-04 7:53 ` Jaskaran Singh 0 siblings, 0 replies; 10+ messages in thread From: jaskaransingh7654321 @ 2019-10-04 7:53 UTC (permalink / raw) 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read @ 2019-10-04 7:53 ` Jaskaran Singh 0 siblings, 0 replies; 10+ messages in thread From: Jaskaran Singh @ 2019-10-04 7:53 UTC (permalink / raw) 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-10-04 7:53 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-10-03 19:30 [Linux-kernel-mentees] [SYZBOT REPORT] divide error in usbtmc_generic_read jaskaransingh7654321 2019-10-03 19:30 ` Jaskaran Singh 2019-10-03 19:56 ` gregkh 2019-10-03 19:56 ` Greg KH 2019-10-04 6:58 ` jaskaransingh7654321 2019-10-04 6:58 ` Jaskaran Singh 2019-10-04 7:21 ` gregkh 2019-10-04 7:21 ` Greg KH 2019-10-04 7:53 ` jaskaransingh7654321 2019-10-04 7:53 ` Jaskaran Singh
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.