[PATCH] ASoC: stm: Prevent potential division by zero in stm32_sai_mclk_round_rate()

[PATCH] ASoC: stm: Prevent potential division by zero in stm32_sai_mclk_round_rate()

[Luo Yifan]

This patch checks if div is less than or equal to zero (div <= 0). If
div is zero or negative, the function returns -EINVAL, ensuring the
division operation (*prate / div) is safe to perform.

Signed-off-by: Luo Yifan <luoyifan@cmss.chinamobile.com>
---
 sound/soc/stm/stm32_sai_sub.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/stm/stm32_sai_sub.c b/sound/soc/stm/stm32_sai_sub.c
index 7bc4a96b7..2570daa3e 100644
--- a/sound/soc/stm/stm32_sai_sub.c
+++ b/sound/soc/stm/stm32_sai_sub.c
@@ -378,8 +378,8 @@ static long stm32_sai_mclk_round_rate(struct clk_hw *hw, unsigned long rate,
 	int div;
 
 	div = stm32_sai_get_clk_div(sai, *prate, rate);
-	if (div < 0)
-		return div;
+	if (div <= 0)
+		return -EINVAL;
 
 	mclk->freq = *prate / div;
 
-- 
2.27.0

Re: [PATCH] ASoC: stm: Prevent potential division by zero in stm32_sai_mclk_round_rate()

[Olivier MOYSAN]

Hi Luo,

On 11/6/24 02:46, Luo Yifan wrote:
> This patch checks if div is less than or equal to zero (div <= 0). If
> div is zero or negative, the function returns -EINVAL, ensuring the
> division operation (*prate / div) is safe to perform.
> 
> Signed-off-by: Luo Yifan <luoyifan@cmss.chinamobile.com>
> ---
>   sound/soc/stm/stm32_sai_sub.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/stm/stm32_sai_sub.c b/sound/soc/stm/stm32_sai_sub.c
> index 7bc4a96b7..2570daa3e 100644
> --- a/sound/soc/stm/stm32_sai_sub.c
> +++ b/sound/soc/stm/stm32_sai_sub.c
> @@ -378,8 +378,8 @@ static long stm32_sai_mclk_round_rate(struct clk_hw *hw, unsigned long rate,
>   	int div;
>   
>   	div = stm32_sai_get_clk_div(sai, *prate, rate);
> -	if (div < 0)
> -		return div;
> +	if (div <= 0)
> +		return -EINVAL;
>   
>   	mclk->freq = *prate / div;
>   

Thanks for your patch. It looks fine, but I think that it has to
be extended.

In CR1 register, MCKDIV = 0 gives the same result as MCKDIV = 1.
But while MCKDIV = 0 is valid, for sure div = 0 is not valid.

I agree that that div = 0 has to be managed as an error
This could be rather handled in stm32_sai_get_clk_div() function itself,
by returning an error, if div is null.
This is relevant as we may also get an error on test "if (input_rate % 
div)".
I suggest to add a specific test and error message to handle this case 
in stm32_sai_get_clk_div().
Something like:
if (!div)) {
	dev_err(&sai->pdev->dev, "Invalid null divider\n");
	return -EINVAL;
}

BRs
Olivier

Re: [PATCH] ASoC: stm: Prevent potential division by zero in stm32_sai_mclk_round_rate()

[Luo Yifan]

Sure, I can submit a new patch with specific tests and error messages added to stm32_sai_get_clk_div().

[PATCH] ASoC: stm: Prevent potential division by zero in stm32_sai_get_clk_div()

[Luo Yifan]

This patch checks if div is less than or equal to zero (div <= 0). If
div is zero or negative, the function returns -EINVAL, ensuring the
division operation is safe to perform.

Signed-off-by: Luo Yifan <luoyifan@cmss.chinamobile.com>
---
 sound/soc/stm/stm32_sai_sub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/stm/stm32_sai_sub.c b/sound/soc/stm/stm32_sai_sub.c
index 7bc4a96b7..43fb1dcb9 100644
--- a/sound/soc/stm/stm32_sai_sub.c
+++ b/sound/soc/stm/stm32_sai_sub.c
@@ -317,7 +317,7 @@ static int stm32_sai_get_clk_div(struct stm32_sai_sub_data *sai,
 	int div;
 
 	div = DIV_ROUND_CLOSEST(input_rate, output_rate);
-	if (div > SAI_XCR1_MCKDIV_MAX(version)) {
+	if (div > SAI_XCR1_MCKDIV_MAX(version) || div <= 0) {
 		dev_err(&sai->pdev->dev, "Divider %d out of range\n", div);
 		return -EINVAL;
 	}
-- 
2.27.0

Re: [PATCH] ASoC: stm: Prevent potential division by zero in stm32_sai_get_clk_div()

[Olivier MOYSAN]

On 11/7/24 02:59, Luo Yifan wrote:
> This patch checks if div is less than or equal to zero (div <= 0). If
> div is zero or negative, the function returns -EINVAL, ensuring the
> division operation is safe to perform.
> 
> Signed-off-by: Luo Yifan <luoyifan@cmss.chinamobile.com>
> ---
>   sound/soc/stm/stm32_sai_sub.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/stm/stm32_sai_sub.c b/sound/soc/stm/stm32_sai_sub.c
> index 7bc4a96b7..43fb1dcb9 100644
> --- a/sound/soc/stm/stm32_sai_sub.c
> +++ b/sound/soc/stm/stm32_sai_sub.c
> @@ -317,7 +317,7 @@ static int stm32_sai_get_clk_div(struct stm32_sai_sub_data *sai,
>   	int div;
>   
>   	div = DIV_ROUND_CLOSEST(input_rate, output_rate);
> -	if (div > SAI_XCR1_MCKDIV_MAX(version)) {
> +	if (div > SAI_XCR1_MCKDIV_MAX(version) || div <= 0) {
>   		dev_err(&sai->pdev->dev, "Divider %d out of range\n", div);
>   		return -EINVAL;
>   	}

Reviewed-by: Olivier Moysan <olivier.moysan@foss.st.com>

thanks

Re: [PATCH] ASoC: stm: Prevent potential division by zero in stm32_sai_mclk_round_rate()

[Mark Brown]

On Wed, 06 Nov 2024 09:46:54 +0800, Luo Yifan wrote:
> This patch checks if div is less than or equal to zero (div <= 0). If
> div is zero or negative, the function returns -EINVAL, ensuring the
> division operation (*prate / div) is safe to perform.
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: stm: Prevent potential division by zero in stm32_sai_mclk_round_rate()
      commit: 63c1c87993e0e5bb11bced3d8224446a2bc62338
[1/1] ASoC: stm: Prevent potential division by zero in stm32_sai_get_clk_div()
      commit: 23569c8b314925bdb70dd1a7b63cfe6100868315

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark