> -----Original Message-----
> From: Pavel Machek <pavel@xxxxxx>
> Sent: Sunday, April 07, 2019 12:20 PM
> To: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> Cc: jacek.anaszewski@xxxxxxxxx; linux-leds@xxxxxxxxxxxxxxx; Ido Schimmel
> <idosch@xxxxxxxxxxxx>
> Subject: Re: [PATCH v1 led] leds: mlxreg: Fix UBSAN warning
>
> On Sun 2019-04-07 09:17:39, Vadim Pasternak wrote:
> >
> >
> > > -----Original Message-----
> > > From: Pavel Machek <pavel@xxxxxx>
> > > Sent: Sunday, April 07, 2019 11:46 AM
> > > To: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> > > Cc: jacek.anaszewski@xxxxxxxxx; linux-leds@xxxxxxxxxxxxxxx; Ido
> > > Schimmel <idosch@xxxxxxxxxxxx>
> > > Subject: Re: [PATCH v1 led] leds: mlxreg: Fix UBSAN warning
> > >
> > > On Sun 2019-04-07 07:59:01, Vadim Pasternak wrote:
> > > > Fix the following UBSAN warning, reported undefined behavior at
> > > > call to
> > > > ror32():
> > > > [ 11.426543] UBSAN: Undefined behaviour in
> ./include/linux/bitops.h:93:33
> > > ...
> > > > The warning is caused by call to ror32(), if the second parameters
> > > > of this function "shift" is zero. In such case UBSAN reports the
> > > > warning for the next expression: (word << (32 - shift).
> > > > Fix adds validation of this parameter ? in case it?s equal zero,
> > > > no need to call rotation function, since value should stay the same.
> > > >
> > > > Fixes: 386570d76f2f ("leds: add driver for support Mellanox regmap
> > > > LEDs for BMC and x86 platform")
> > > > Reported-by: Ido Schimmel <idosch@xxxxxxxxxxxx>
> > > > Signed-off-by: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> > >
> > > Actually, rotating zero bits seems pretty valid operation. Could we
> > > get that fixed, instead?
> >
> > Hi Pavel,
>
> Please fix it as:
>
> > UBSAN warns in case of call ror32(x, 0):
> > static inline __u32 ror32(__u32 word, __u32 shift) {
> + if (!shift) return word;
> > return (word >> shift) | (word << (32 - shift)); }
> >
In such case I suppose it should be fixed also in ror64, ror16, ror8.
> > The ror32() shift parameter zero causes rotation right of 32 for u32
> > type.
> >
> > For example UBSAN will produce the warning for the below code:
> > u32 value = <some value>;
> > value <<= 32;
>
> Thanks,
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
