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

Chancel Liu posted 1 patch 1 week, 3 days ago
There is a newer version of this series
sound/soc/fsl/fsl_sai.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] ASoC: fsl_sai: Fix 32 slots TDM broken by integer shift UB in xMR write
Posted by Chancel Liu 1 week, 3 days ago
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
Posted by Christophe Leroy (CS GROUP) 1 week, 3 days ago

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;
>   }

[PATCH v2] ASoC: fsl_sai: Fix 32 slots TDM broken by integer shift UB in xMR write
Posted by chancel.liu@oss.nxp.com 1 week ago
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