lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 14 Jun 2022 12:49:38 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     "Fabio M. De Francesco" <fmdefrancesco@...il.com>
Cc:     Eugeniu Rosca <erosca@...adit-jv.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, Mark Brown <broonie@...nel.org>,
        alsa-devel@...a-project.org, linux-kernel@...r.kernel.org,
        syzbot+205eb15961852c2c5974@...kaller.appspotmail.com,
        naveenkumar.sunkari@...bosch.com,
        Eugeniu Rosca <roscaeugeniu@...il.com>
Subject: Re: [PATCH] ALSA: pcm: Test for "silence" field in struct "pcm_format_data"

On Tue, 14 Jun 2022 12:43:16 +0200,
Fabio M. De Francesco wrote:
> 
> On martedì 14 giugno 2022 11:58:51 CEST Eugeniu Rosca wrote:
> > Hello Fabio, hello All,
> > 
> > On Sa, Apr 09, 2022 at 03:26:55 +0200, Fabio M. De Francesco wrote:
> > > Syzbot reports "KASAN: null-ptr-deref Write in
> > > snd_pcm_format_set_silence".[1]
> > > 
> > > It is due to missing validation of the "silence" field of struct
> > > "pcm_format_data" in "pcm_formats" array.
> > > 
> > > Add a test for valid "pat" and, if it is not so, return -EINVAL.
> > > 
> > > [1] https://lore.kernel.org/lkml/
> 000000000000d188ef05dc2c7279@...gle.com/
> > > 
> > > Reported-and-tested-by: 
> syzbot+205eb15961852c2c5974@...kaller.appspotmail.com
> > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@...il.com>
> > > ---
> > > 
> > > I wasn't able to figure out the commit for the "Fixes:" tag. If this 
> patch
> > > is good, can someone please help with providing this missing 
> information?
> > > 
> > >  sound/core/pcm_misc.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c
> > > index 4866aed97aac..5588b6a1ee8b 100644
> > > --- a/sound/core/pcm_misc.c
> > > +++ b/sound/core/pcm_misc.c
> > > @@ -433,7 +433,7 @@ int snd_pcm_format_set_silence(snd_pcm_format_t 
> format, void *data, unsigned int
> > >  		return 0;
> > >  	width = pcm_formats[(INT)format].phys; /* physical width */
> > >  	pat = pcm_formats[(INT)format].silence;
> > > -	if (! width)
> > > +	if (!width || !pat)
> > >  		return -EINVAL;
> > >  	/* signed or 1 byte data */
> > >  	if (pcm_formats[(INT)format].signd == 1 || width <= 8) {
> > 
> > JFYI, PVS-Studio 7.19 reports:
> > 
> > sound/core/pcm_misc.c	409	warn	V560 A part of 
> conditional expression is always false: !pat.
> 
> Sorry, I assumed (wrongly!) that when we have
> 
> static const struct pcm_format_data 
> pcm_formats[(INT)SNDRV_PCM_FORMAT_LAST+1] = {
> 	[SNDRV_PCM_FORMAT_S8] = {
> 		.width = 8, .phys = 8, .le = -1, .signd = 1,
> 		.silence = {},
> 	},
> 	[snip]
> 	/* FIXME: the following two formats are not defined properly yet 
> */
> 	[SNDRV_PCM_FORMAT_MPEG] = {
> 		.le = -1, .signd = -1,
> 	},
> 	[SNDRV_PCM_FORMAT_GSM] = {
> 		.le = -1, .signd = -1,
> 	},
> 
> pointer "silence", and then "pat", must be NULL.

Oh right, those are missing ones.  I haven't realized that those
formats are allowed by PCM OSS layer.

Practically seen, those formats have never been used in reality, and
we may consider dropping them completely to plug such holes...


Takashi

Powered by blists - more mailing lists