scsi: aacraid: address UBSAN warning regression
authorArnd Bergmann <arnd@arndb.de>
Tue, 28 Nov 2017 13:25:25 +0000 (14:25 +0100)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sun, 4 Nov 2018 13:52:42 +0000 (14:52 +0100)
commitc750773f3942863a37e7415934a646a0da7b1040
tree872a384c86b1bc6232922de3b0198036528a9833
parent3b6c6d9cbdea50aed8f6bf22575461d8009e3951
scsi: aacraid: address UBSAN warning regression

[ Upstream commit d18539754d97876503275efc7d00a1901bb0cfad ]

As reported by Meelis Roos, my previous patch causes an incorrect
calculation of the timeout, through an undefined signed integer
overflow:

[   12.228155] UBSAN: Undefined behaviour in drivers/scsi/aacraid/commsup.c:2514:49
[   12.228229] signed integer overflow:
[   12.228283] 964297611 * 250 cannot be represented in type 'long int'

The problem is that doing a multiplication with HZ first and then
dividing by USEC_PER_SEC worked correctly for 32-bit microseconds,
but not for 32-bit nanoseconds, which would require up to 41 bits.

This reworks the calculation to first convert the nanoseconds into
jiffies, which should give us the same result as before and not overflow.

Unfortunately I did not understand the exact intention of the algorithm,
in particular the part where we add half a second, so it's possible that
there is still a preexisting problem in this function. I added a comment
that this would be handled more nicely using usleep_range(), which
generally works better for waking up at a particular time than the
current schedule_timeout() based implementation. I did not feel
comfortable trying to implement that without being sure what the
intent is here though.

Fixes: 820f18865912 ("scsi: aacraid: use timespec64 instead of timeval")
Tested-by: Meelis Roos <mroos@linux.ee>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
drivers/scsi/aacraid/commsup.c