[PATCH] ALSA: hdsp: Fix divide-by-zero in DDS offset control

[PATCH] ALSA: hdsp: Fix divide-by-zero in DDS offset control

[songxiebing]

From: Bob Song <songxiebing@kylinos.cn>

hdsp_set_dds_offset() computes the target rate as
system_sample_rate + offset_hz without validating the result.  A user
can set the DDS offset to -system_sample_rate through the control API,
making rate zero and causing div_u64() in hdsp_set_dds_value() to
divide by zero.

Reject non-positive rates before programming the DDS value.

Fixes: e4b6088c8cf1 ("[ALSA] hdsp - Add support for fine tuning of sample rate support to HDSP 9632")
Cc: <stable@vger.kernel.org>
Signed-off-by: Bob Song <songxiebing@kylinos.cn>
---
 sound/pci/rme9652/hdsp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
index 7dce548b9f36..694816f966d2 100644
--- a/sound/pci/rme9652/hdsp.c
+++ b/sound/pci/rme9652/hdsp.c
@@ -2802,6 +2802,9 @@ static int hdsp_dds_offset(struct hdsp *hdsp)
 static int hdsp_set_dds_offset(struct hdsp *hdsp, int offset_hz)
 {
 	int rate = hdsp->system_sample_rate + offset_hz;
+
+	if (rate <= 0)
+		return -EINVAL;
 	hdsp_set_dds_value(hdsp, rate);
 	return 0;
 }
-- 
2.25.1

Re: [PATCH] ALSA: hdsp: Fix divide-by-zero in DDS offset control

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ALSA: hdsp: incomplete offset validation in hdsp_set_dds_offset()
--

commit a91b0358087c1f7f09a07e8d353cc380f3a7e223
Author: Bob Song <songxiebing@kylinos.cn>

ALSA: hdsp: Fix divide-by-zero in DDS offset control

This commit updates hdsp_set_dds_offset() to reject non-positive target
sample rates with -EINVAL before calling hdsp_set_dds_value() to prevent a
divide-by-zero in div_u64().

> diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
> index 7dce548b9f363..694816f966d29 100644
> --- a/sound/pci/rme9652/hdsp.c
> +++ b/sound/pci/rme9652/hdsp.c
> @@ -2802,6 +2802,9 @@ static int hdsp_dds_offset(struct hdsp *hdsp)
>  static int hdsp_set_dds_offset(struct hdsp *hdsp, int offset_hz)
>  {
>  	int rate = hdsp->system_sample_rate + offset_hz;
> +
> +	if (rate <= 0)
> +		return -EINVAL;

[Severity: Medium]
Is checking rate <= 0 sufficient to validate offset_hz here?

When snd_hdsp_put_dds_offset() calls hdsp_set_dds_offset(), offset_hz comes
from ucontrol->value.integer.value[0] without enforcing the [-5000, 5000]
range declared in snd_hdsp_info_dds_offset():

sound/pci/rme9652/hdsp.c:snd_hdsp_info_dds_offset() {
	...
	uinfo->value.integer.min = -5000;
	uinfo->value.integer.max = 5000;
	return 0;
}

First, adding an unvalidated positive offset_hz to hdsp->system_sample_rate
can overflow a signed 32-bit int.

Second, a positive rate in the range 1..24414 (for example, passing
offset_hz = 1 - hdsp->system_sample_rate) still passes the rate <= 0 check
and reaches hdsp_set_dds_value():

sound/pci/rme9652/hdsp.c:hdsp_set_dds_value() {
	...
	n = DDS_NUMERATOR;
	n = div_u64(n, rate);
	/* n should be less than 2^32 for being written to FREQ register */
	snd_BUG_ON(n >> 32);
	/* HDSP_freqReg and HDSP_resetPointer are the same, so keep the DDS
	   value to write it after a reset */
	hdsp->dds_value = n;
	hdsp_write(hdsp, HDSP_freqReg, hdsp->dds_value);
}

Because DDS_NUMERATOR is 104857600000000ULL, any rate in 1..24414 produces
n >= 2^32, which triggers snd_BUG_ON(n >> 32) and truncates n when writing
hdsp->dds_value and HDSP_freqReg. Offsets outside [-5000, 5000] can also
cross the 56000 or 112000 thresholds in hdsp_set_dds_value(), whereas
hdsp_dds_offset() chooses the 2x or 4x multiplier based on
hdsp->system_sample_rate.

In addition, when hdsp_set_dds_offset() returns -EINVAL, its caller
snd_hdsp_put_dds_offset() drops the error code:

sound/pci/rme9652/hdsp.c:snd_hdsp_put_dds_offset() {
	...
	if (val != hdsp_dds_offset(hdsp))
		change = (hdsp_set_dds_offset(hdsp, val) == 0) ? 1 : 0;
	...
	return change;
}

Should hdsp_set_dds_offset() validate that offset_hz is within
[-5000, 5000] before computing rate, and should snd_hdsp_put_dds_offset()
propagate negative error codes from hdsp_set_dds_offset()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104083045.1939479-1-songxiebing@kylinos.cn?part=1

[PATCH v2] ALSA: hdsp: Validate DDS offset range and propagate errors

[songxiebing]

From: Bob Song <songxiebing@kylinos.cn>

hdsp_set_dds_offset() computes the target rate as
system_sample_rate + offset_hz without validating the result.  A user
can set the DDS offset to -system_sample_rate through the control API,
making rate zero and causing div_u64() in hdsp_set_dds_value() to
divide by zero.  Larger offsets can also overflow the rate calculation
or push the DDS value out of range.

Validate offset_hz against the [-5000, 5000] range advertised by the
control, keep the non-positive rate check as a safety net, and
propagate the error from snd_hdsp_put_dds_offset() instead of
reporting success.

Fixes: e4b6088c8cf1 ("[ALSA] hdsp - Add support for fine tuning of sample rate support to HDSP 9632")
Cc: <stable@vger.kernel.org>
Signed-off-by: Bob Song <songxiebing@kylinos.cn>
---
 sound/pci/rme9652/hdsp.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
index 7dce548b9f36..467121a5fb4f 100644
--- a/sound/pci/rme9652/hdsp.c
+++ b/sound/pci/rme9652/hdsp.c
@@ -2768,6 +2768,9 @@ static int snd_hdsp_get_adat_sync_check(struct snd_kcontrol *kcontrol, struct sn
 	return 0;
 }
 
+#define HDSP_DDS_OFFSET_MIN	-5000
+#define HDSP_DDS_OFFSET_MAX	5000
+
 #define HDSP_DDS_OFFSET(xname, xindex) \
 { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, \
   .name = xname, \
@@ -2801,7 +2804,15 @@ static int hdsp_dds_offset(struct hdsp *hdsp)
 
 static int hdsp_set_dds_offset(struct hdsp *hdsp, int offset_hz)
 {
-	int rate = hdsp->system_sample_rate + offset_hz;
+	int rate;
+
+	if (offset_hz < HDSP_DDS_OFFSET_MIN ||
+	    offset_hz > HDSP_DDS_OFFSET_MAX)
+		return -EINVAL;
+
+	rate = hdsp->system_sample_rate + offset_hz;
+	if (rate <= 0)
+		return -EINVAL;
 	hdsp_set_dds_value(hdsp, rate);
 	return 0;
 }
@@ -2810,8 +2821,8 @@ static int snd_hdsp_info_dds_offset(struct snd_kcontrol *kcontrol, struct snd_ct
 {
 	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
 	uinfo->count = 1;
-	uinfo->value.integer.min = -5000;
-	uinfo->value.integer.max = 5000;
+	uinfo->value.integer.min = HDSP_DDS_OFFSET_MIN;
+	uinfo->value.integer.max = HDSP_DDS_OFFSET_MAX;
 	return 0;
 }
 
@@ -2833,10 +2844,14 @@ static int snd_hdsp_put_dds_offset(struct snd_kcontrol *kcontrol, struct snd_ctl
 		return -EBUSY;
 	val = ucontrol->value.integer.value[0];
 	guard(spinlock_irq)(&hdsp->lock);
-	if (val != hdsp_dds_offset(hdsp))
-		change = (hdsp_set_dds_offset(hdsp, val) == 0) ? 1 : 0;
-	else
+	if (val != hdsp_dds_offset(hdsp)) {
+		change = hdsp_set_dds_offset(hdsp, val);
+		if (change < 0)
+			return change;
+		change = 1;
+	} else {
 		change = 0;
+	}
 	return change;
 }
 
-- 
2.25.1

Re: [PATCH] ALSA: hdsp: Fix divide-by-zero in DDS offset control

[Takashi Iwai]

On Wed, 04 Nov 2026 09:30:45 +0100,
songxiebing wrote:
> 
> From: Bob Song <songxiebing@kylinos.cn>
> 
> hdsp_set_dds_offset() computes the target rate as
> system_sample_rate + offset_hz without validating the result.  A user
> can set the DDS offset to -system_sample_rate through the control API,
> making rate zero and causing div_u64() in hdsp_set_dds_value() to
> divide by zero.
> 
> Reject non-positive rates before programming the DDS value.
> 
> Fixes: e4b6088c8cf1 ("[ALSA] hdsp - Add support for fine tuning of sample rate support to HDSP 9632")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Bob Song <songxiebing@kylinos.cn>
> ---
>  sound/pci/rme9652/hdsp.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
> index 7dce548b9f36..694816f966d2 100644
> --- a/sound/pci/rme9652/hdsp.c
> +++ b/sound/pci/rme9652/hdsp.c
> @@ -2802,6 +2802,9 @@ static int hdsp_dds_offset(struct hdsp *hdsp)
>  static int hdsp_set_dds_offset(struct hdsp *hdsp, int offset_hz)
>  {
>  	int rate = hdsp->system_sample_rate + offset_hz;
> +
> +	if (rate <= 0)
> +		return -EINVAL;
>  	hdsp_set_dds_value(hdsp, rate);
>  	return 0;
>  }

The caller doesn't check the error case properly.
Please fix the caller side, too.


thanks,

Takashi