From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 448DCC43334 for ; Tue, 14 Jun 2022 10:27:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1355261AbiFNK1e (ORCPT ); Tue, 14 Jun 2022 06:27:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42948 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229853AbiFNK1d (ORCPT ); Tue, 14 Jun 2022 06:27:33 -0400 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 820C91838D for ; Tue, 14 Jun 2022 03:27:32 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 3CB611F88B; Tue, 14 Jun 2022 10:27:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1655202451; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=V1c4KqI75Xq0osGRVechadykZB657HEVNEGp9LJkIc4=; b=cNx53KGmbn6nMHd5CEj3v2EWhCikpzJh3VqMQc1hTO5UFMHGm5B4Ths0iOlGqMtufajtGr 5wuKcCynC/P55u1CLdVTVZvwMw2pw3tgu5311t6wdH/LJbdM0+X+5K1FyJHAtO8ouOhMgv T7BEy1YZ6KoVS99Cq68AkcKmQplCpYs= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1655202451; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=V1c4KqI75Xq0osGRVechadykZB657HEVNEGp9LJkIc4=; b=slERjjyzNG8rnCyJlLq0795WKEpFYxJ9s1V3m5t6d/WuTASmBhlz1COoBbBipKsaK9IMrC prDe3PCPS34p+aDQ== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 256AA1361C; Tue, 14 Jun 2022 10:27:31 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id Ao3NCJNiqGK4VQAAMHmgww (envelope-from ); Tue, 14 Jun 2022 10:27:31 +0000 Date: Tue, 14 Jun 2022 12:27:30 +0200 Message-ID: <8735g7r14d.wl-tiwai@suse.de> From: Takashi Iwai To: Eugeniu Rosca Cc: "Fabio M. De Francesco" , Jaroslav Kysela , Takashi Iwai , Mark Brown , , , , , Eugeniu Rosca Subject: Re: [PATCH] ALSA: pcm: Test for "silence" field in struct "pcm_format_data" In-Reply-To: <20220614095851.GA4199@lxhi-065> References: <20220409012655.9399-1-fmdefrancesco@gmail.com> <20220614095851.GA4199@lxhi-065> User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0 MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 14 Jun 2022 11:58:51 +0200, 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@google.com/ > > > > Reported-and-tested-by: syzbot+205eb15961852c2c5974@syzkaller.appspotmail.com > > Signed-off-by: Fabio M. De Francesco > > --- > > > > 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. > > I haven't fully validated the finding, but it appears to be legit, > since the pointer variable (as opposed to the contents behind the > pointer) is always non-null, hence !pat always evaluating to false. > > If the above is true, then the patch likely hasn't introduced any > regression, but also likely hasn't fixed the original KASAN problem. > > Or are there alternative views? Indeed the fix looks bogus, and maybe better to revert. Looking at the original syzkaller report again, it points rather to the *write* at the address 1, and it means not the source (silence[]) but the target pointer (data) is invalid; i.e. it's a problem in the caller side, likely some race between the OSS temporary buffer removal and other operation. Thanks for checking this. Takashi