RE: Re: [PATCH] ASoC: fsl_sai: Fix 32 slots TDM broken by integer shift UB in xMR write

[Chancel Liu]

> > When configuring 32 slots TDM (channels == slots == 32), the xMR (Mask
> > Register) write used:
> > ~0UL - ((1 << min(channels, slots)) - 1)
> >
> > The literal '1' is a signed 32-bit int. Shifting it by 32 positions is
> > undefined behaviour which may set this register to 0xFFFFFFFF, masking
> > all 32 slots.
> >
> > Use 1ULL so the shift is carried out in 64 bits. For 32 slots this
> > produces a zero mask after truncation to the 32-bit register:
> > ~0ULL - ((1ULL << 32) - 1)
> >    = 0xFFFFFFFFFFFFFFFF - (0x100000000 - 1)
> >    = 0xFFFFFFFFFFFFFFFF - 0xFFFFFFFF
> >    = 0xFFFFFFFF00000000
> >    -> Truncates to 0x00000000
> > Behaviour for fewer than 32 slots is unchanged.
> 
> Why not use macro GENMASK_U32() instead ?
>

Thanks for this reminder. OK, I will switch to the clearer and safer
GENMASK_U32() macro:
	regmap_write(sai->regmap, FSL_SAI_xMR(tx),
		     ~GENMASK_U32(min(channels, slots) - 1, 0));

Regards, 
Chancel Liu

> >
> > Fixes: 770f58d7d2c5 ("ASoC: fsl_sai: Support multiple data channel
> > enable bits")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Chancel Liu <chancel.liu@xxxxxxx>
> > ---
> >   sound/soc/fsl/fsl_sai.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index
> > d6dd95680892..821e3bd51b6e 100644
> > --- a/sound/soc/fsl/fsl_sai.c
> > +++ b/sound/soc/fsl/fsl_sai.c
> > @@ -797,7 +797,7 @@ static int fsl_sai_hw_params(struct
> snd_pcm_substream *substream,
> >                                  FSL_SAI_CR4_FSD_MSTR,
> > FSL_SAI_CR4_FSD_MSTR);
> >
> >       regmap_write(sai->regmap, FSL_SAI_xMR(tx),
> > -                  ~0UL - ((1 << min(channels, slots)) - 1));
> > +                  ~0ULL - ((1ULL << min(channels, slots)) - 1));
> >
> >       return 0;
> >   }