On martedì 14 giugno 2022 12:49:38 CEST Takashi Iwai wrote:
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.
000000000000d188ef05dc2c7279@google.com/
Reported-and-tested-by:
syzbot+205eb15961852c2c5974@syzkaller.appspotmail.com
Signed-off-by: Fabio M. De Francesco fmdefrancesco@gmail.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...
Does it imply that my argument is correct or my "fix" can't yet catch those missing ones?
Besides the question above, I want to notice that we have one more /* FIXME */ entry...
/* FIXME: the following format is not defined properly yet */ [SNDRV_PCM_FORMAT_SPECIAL] = { .le = -1, .signd = -1, },
If you want I can get rid of those three entries if you confirm they can safely be deleted. In a second patch I can also remove that unnecessary check for valid "pat".
Please let me know.
Thanks,
Fabio