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

[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.

Fixes: 770f58d7d2c5 ("ASoC: fsl_sai: Support multiple data channel enable bits")
Cc: stable@vger.kernel.org
Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
---
 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;
 }
-- 
2.50.1

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

[Christophe Leroy (CS GROUP)]

Le 29/05/2026 à 10:50, Chancel Liu a écrit :
> 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 ?

> 
> Fixes: 770f58d7d2c5 ("ASoC: fsl_sai: Support multiple data channel enable bits")
> Cc: stable@vger.kernel.org
> Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
> ---
>   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;
>   }

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@vger.kernel.org
> > Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
> > ---
> >   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;
> >   }

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

[chancel.liu]

From: Chancel Liu <chancel.liu@nxp.com>

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 GENMASK_U32() macro instead. For 32 slots this produces a zero mask:
~GENMASK_U32(31, 0) = ~0xFFFFFFFF = 0x00000000
Behaviour for fewer than 32 slots is unchanged.

Fixes: 770f58d7d2c5 ("ASoC: fsl_sai: Support multiple data channel enable bits")
Cc: stable@vger.kernel.org
Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
---
Changes in v2
- Use GENMASK_U32() macro instead to make it clearer and safer

 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 821e3bd51b6e..9661602b53c5 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),
-		     ~0ULL - ((1ULL << min(channels, slots)) - 1));
+		     ~GENMASK_U32(min(channels, slots) - 1, 0));

 	return 0;
 }
--
2.50.1

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

[Chancel Liu (OSS)]

Sorry, please ignore v2, it can't be applied successfully.
Will send v3 shortly.

Regards, 
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 GENMASK_U32() macro instead. For 32 slots this produces a zero mask:
> ~GENMASK_U32(31, 0) = ~0xFFFFFFFF = 0x00000000 Behaviour for fewer than 32
> slots is unchanged.
> 
> Fixes: 770f58d7d2c5 ("ASoC: fsl_sai: Support multiple data channel enable
> bits")
> Cc: stable@vger.kernel.org
> Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
> ---
> Changes in v2
> - Use GENMASK_U32() macro instead to make it clearer and safer
> 
>  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
> 821e3bd51b6e..9661602b53c5 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),
> -		     ~0ULL - ((1ULL << min(channels, slots)) - 1));
> +		     ~GENMASK_U32(min(channels, slots) - 1, 0));
> 
>  	return 0;
>  }
> --
> 2.50.1