On Fri, 24 Jan 2014, James Bottomley wrote:
> On Fri, 2014-01-24 at 11:34 -0500, Mikulas Patocka wrote: > > On alpha, USER_HZ may be higher than HZ. This results in integer overflow > > in MULDIV. > > > > Signed-off-by: Mikulas Patocka <mpato...@redhat.com> > > Cc: sta...@vger.kernel.org > > > > --- > > drivers/scsi/sg.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > Index: linux-3.13/drivers/scsi/sg.c > > =================================================================== > > --- linux-3.13.orig/drivers/scsi/sg.c 2014-01-20 21:44:02.000000000 > > +0100 > > +++ linux-3.13/drivers/scsi/sg.c 2014-01-24 17:28:02.000000000 +0100 > > @@ -856,8 +856,10 @@ sg_ioctl(struct file *filp, unsigned int > > return result; > > if (val < 0) > > return -EIO; > > +#if USER_HZ < HZ > > if (val >= MULDIV (INT_MAX, USER_HZ, HZ)) > > val = MULDIV (INT_MAX, USER_HZ, HZ); > > +#endif > > Is this really a problem worth fixing? Is USER_HZ ever greater than HZ? Yes. The integer overflow results in unpredictable value and the timeout is limited to that value. > > sfp->timeout_user = val; > > If it can happen, this needs fixing as well: > > > sfp->timeout = MULDIV (val, HZ, USER_HZ); This line is correct. Note, that here the arguments HZ and USER_HZ are swapped. Here, you divide val by USER_HZ and multiply it by HZ, so there is no oveflow if USER_HZ > HZ. There could be overflow if USER_HZ < HZ, but the above "if" condition avoids that. > any val that would divide to less than 1 will now set the timeout to > zero, which will cause the queue default timeout to be applied instead. > > I'd fix it in the macro rather than having code peppered with #ifs You can't easily fix it in the macro: "MULDIV (INT_MAX, USER_HZ, HZ)" is undefined value if USER_HZ > HZ, it is not specified what the macro should return in this case. > James Mikulas -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
