Re: [PATCH] mptsas: fix undefined behaviour of a shift of an int by more than 31 places

From: Hannes Reinecke
Date: Thu May 09 2019 - 11:31:54 EST


On 5/8/19 4:24 PM, James Bottomley wrote:
On Wed, 2019-05-08 at 14:07 +0100, Colin Ian King wrote:
On 05/05/2019 04:34, James Bottomley wrote:
On Sat, 2019-05-04 at 17:40 +0100, Colin King wrote:
From: Colin Ian King <colin.king@xxxxxxxxxxxxx>

Currently the shift of int value 1 by more than 31 places can
result in undefined behaviour. Fix this by making the 1 a ULL
value before the shift operation.

Fusion SAS is pretty ancient. I thought the largest one ever
produced had four phys, so how did you produce the overflow?

This was an issue found by static analysis with Coverity; so I guess
won't happen in the wild, in which case the patch could be ignored.

The point I was more making is that if we thought this could ever
happen in practice, we'd need more error handling than simply this:
we'd be setting the phy_bitmap to zero which would be every bit as bad
as some random illegal value.

Thing is, mptsas is used as the default emulation in VMWare, and that does allow you to do some pretty weird configurations (I've found myself fixing a bug with SATA hotplug on mptsas once ...).
So I wouldn't discard this issue out of hand.

Cheers,

Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@xxxxxxx +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 NÃrnberg
GF: Felix ImendÃrffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG NÃrnberg)