On Sun 2019-04-07 09:34:22, Vadim Pasternak wrote:
>
>
> > -----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.
I guess so, yes.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Attachment:
signature.asc
Description: Digital signature
