[PATCH] ALSA: pcm: oss: Compare max buffer size in bytes

[PATCH] ALSA: pcm: oss: Compare max buffer size in bytes

[Takashi Iwai]

We introduced the upper-limit check of the buffer size for PCM OSS to
1MB bytes, but at one place (for PCM OSS plugin-base), the check was
applied at a wrong position: the size is compared before scaling down
to byte size, so it's limiting essentially to 1MB bits, i.e. 128kB
bytes, instead.

Although 128kB is enough in practical use cases, it's better to align
with other places for the code consistency.  So this patch moves the
check to the right position after conversion to bytes, so that it can
limit to the correct size, 1MB bytes.

Fixes: efb6402c3c4a ("ALSA: oss: Fix PCM OSS buffer allocation overflow")
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/oss/pcm_plugin.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/core/oss/pcm_plugin.c b/sound/core/oss/pcm_plugin.c
index 3de713779993..c5b3c67345ee 100644
--- a/sound/core/oss/pcm_plugin.c
+++ b/sound/core/oss/pcm_plugin.c
@@ -48,12 +48,12 @@ static int snd_pcm_plugin_alloc(struct snd_pcm_plugin *plugin, snd_pcm_uframes_t
 	if (width < 0)
 		return width;
 	size = array3_size(frames, format->channels, width);
-	/* check for too large period size once again */
-	if (size > 1024 * 1024)
-		return -ENOMEM;
 	if (snd_BUG_ON(size % 8))
 		return -ENXIO;
 	size /= 8;
+	/* check for too large period size once again */
+	if (size > 1024 * 1024)
+		return -ENOMEM;
 	if (plugin->buf_frames < frames) {
 		kvfree(plugin->buf);
 		plugin->buf = kvzalloc(size, GFP_KERNEL);
-- 
2.53.0

Re: [PATCH] ALSA: pcm: oss: Compare max buffer size in bytes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] ALSA: oss: array3_size() overflow hits snd_BUG_ON() in
  snd_pcm_plugin_alloc()
--

commit 97a1260d8f9d7c584f3610ce297a74916707f416
Author: Takashi Iwai <tiwai@suse.de>

ALSA: pcm: oss: Compare max buffer size in bytes

This commit moves the 1MB upper-limit check in snd_pcm_plugin_alloc()
to after the bit-to-byte division so the limit is enforced in bytes
rather than bits.

> diff --git a/sound/core/oss/pcm_plugin.c b/sound/core/oss/pcm_plugin.c
> index 3de7137799936..c5b3c67345eeb 100644
> --- a/sound/core/oss/pcm_plugin.c
> +++ b/sound/core/oss/pcm_plugin.c
> @@ -48,12 +48,12 @@ static int snd_pcm_plugin_alloc(struct snd_pcm_plugin *plugin, snd_pcm_uframes_t
>  	if (width < 0)
>  		return width;
>  	size = array3_size(frames, format->channels, width);
> -	/* check for too large period size once again */
> -	if (size > 1024 * 1024)
> -		return -ENOMEM;
>  	if (snd_BUG_ON(size % 8))
>  		return -ENXIO;
>  	size /= 8;
> +	/* check for too large period size once again */
> +	if (size > 1024 * 1024)
> +		return -ENOMEM;

[Severity: Medium]
Can a multiplication overflow in array3_size() now trigger snd_BUG_ON()?
When array3_size() overflows, it saturates to SIZE_MAX, where SIZE_MAX % 8
is 7.  With the size > 1024 * 1024 check moved past snd_BUG_ON(size % 8),
a saturated size hits the WARN_ON() in snd_BUG_ON() when CONFIG_SND_DEBUG
is enabled and returns -ENXIO instead of -ENOMEM.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112123430.684056-1-tiwai@suse.de?part=1

Re: [PATCH] ALSA: pcm: oss: Compare max buffer size in bytes

[Takashi Iwai]

On Thu, 12 Nov 2026 13:49:21 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] ALSA: oss: array3_size() overflow hits snd_BUG_ON() in
>   snd_pcm_plugin_alloc()
> --
> 
> commit 97a1260d8f9d7c584f3610ce297a74916707f416
> Author: Takashi Iwai <tiwai@suse.de>
> 
> ALSA: pcm: oss: Compare max buffer size in bytes
> 
> This commit moves the 1MB upper-limit check in snd_pcm_plugin_alloc()
> to after the bit-to-byte division so the limit is enforced in bytes
> rather than bits.
> 
> > diff --git a/sound/core/oss/pcm_plugin.c b/sound/core/oss/pcm_plugin.c
> > index 3de7137799936..c5b3c67345eeb 100644
> > --- a/sound/core/oss/pcm_plugin.c
> > +++ b/sound/core/oss/pcm_plugin.c
> > @@ -48,12 +48,12 @@ static int snd_pcm_plugin_alloc(struct snd_pcm_plugin *plugin, snd_pcm_uframes_t
> >  	if (width < 0)
> >  		return width;
> >  	size = array3_size(frames, format->channels, width);
> > -	/* check for too large period size once again */
> > -	if (size > 1024 * 1024)
> > -		return -ENOMEM;
> >  	if (snd_BUG_ON(size % 8))
> >  		return -ENXIO;
> >  	size /= 8;
> > +	/* check for too large period size once again */
> > +	if (size > 1024 * 1024)
> > +		return -ENOMEM;
> 
> [Severity: Medium]
> Can a multiplication overflow in array3_size() now trigger snd_BUG_ON()?
> When array3_size() overflows, it saturates to SIZE_MAX, where SIZE_MAX % 8
> is 7.  With the size > 1024 * 1024 check moved past snd_BUG_ON(size % 8),
> a saturated size hits the WARN_ON() in snd_BUG_ON() when CONFIG_SND_DEBUG
> is enabled and returns -ENXIO instead of -ENOMEM.

It's is no practical problem: the size % 8 check is only a sanity
check and this must not be triggered at all.  If it's triggered, it's
a real problem we should catch.

The calculated size can't overflow the integer size, either, because
the passed frame size must have been already limited to the driver's
max buffer size.


Takashi