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 40080CE79AD for ; Wed, 20 Sep 2023 07:05:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233551AbjITHFO (ORCPT ); Wed, 20 Sep 2023 03:05:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60742 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232386AbjITHFL (ORCPT ); Wed, 20 Sep 2023 03:05:11 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EF33C92 for ; Wed, 20 Sep 2023 00:05:05 -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-out1.suse.de (Postfix) with ESMTPS id 953072251C; Wed, 20 Sep 2023 07:05:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1695193504; 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=YtpT9rmsvNdD25C52w/dsFnFom7gmr6h8hTnMT9pppA=; b=VUdNbsptH6xtu8SuSD1phRT0W/65IBdC356rMtSp3IQLekQbLsJxbwDlmZjGBov3oaLqRB c/RiR3ah/OzaVhc+F0ig/I9Vlzp645LprpNEG8znmpmzCYGfbSr5pNmFf+wWcvMdtjCNkN H31bI+i0XvPIYCvOeI3NHFp31r0uEI4= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1695193504; 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=YtpT9rmsvNdD25C52w/dsFnFom7gmr6h8hTnMT9pppA=; b=5Sa7R9OcmwNsSJDDy17+/1xNUt5/Qo67Eu1e4a9fpmRM2Y3HvpWjCY+kmyG0My/2mWR799 rLVDYboGJxWv1GBA== 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 523051333E; Wed, 20 Sep 2023 07:05:04 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id iygqE6CZCmXUJgAAMHmgww (envelope-from ); Wed, 20 Sep 2023 07:05:04 +0000 Date: Wed, 20 Sep 2023 09:05:03 +0200 Message-ID: <871qetwcio.wl-tiwai@suse.de> From: Takashi Iwai To: Ma Ke Cc: perex@perex.cz, tiwai@suse.com, mhocko@suse.com, akpm@linux-foundation.org, mgorman@techsingularity.net, 42.hyeyoo@gmail.com, surenb@google.com, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] ALSA: pcm: oss: Fix race at SNDCTL_DSP_SETTRIGGER There is a small race window at snd_pcm_oss_set_trigger() that is called from OSS PCM SNDCTL_DSP_SETTRIGGER ioctl; namely the function calls snd_pcm_oss_make_ready() at first, then takes the params_lock mutex for the rest. When the stream is set up again by another thread between them, it leads to inconsistency, and may result in unexpected results such as NULL dereference of OSS buffer as a fuzzer spotted recently. In-Reply-To: <20230920023536.3535439-1-make_ruc2021@163.com> References: <20230920023536.3535439-1-make_ruc2021@163.com> 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 Wed, 20 Sep 2023 04:35:36 +0200, Ma Ke wrote: > > The fix is simply to cover snd_pcm_oss_make_ready() call into the same > params_lock mutex with snd_pcm_oss_make_ready_locked() variant. > > Signed-off-by: Ma Ke The patch subject needs to be fixed. The subject line can't be that long. It must be concise. Put more text in the patch description instead. About the code change: the error handling is different after the patch. The current code returns an error immediately while your patch does "goto _skip1" etc, which doesn't abort but continues. thanks, Takashi > --- > sound/core/oss/pcm_oss.c | 16 ++++++---------- > 1 file changed, 6 insertions(+), 10 deletions(-) > > diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c > index 728c211142d1..9a830aeeba63 100644 > --- a/sound/core/oss/pcm_oss.c > +++ b/sound/core/oss/pcm_oss.c > @@ -2083,21 +2083,14 @@ static int snd_pcm_oss_set_trigger(struct snd_pcm_oss_file *pcm_oss_file, int tr > psubstream = pcm_oss_file->streams[SNDRV_PCM_STREAM_PLAYBACK]; > csubstream = pcm_oss_file->streams[SNDRV_PCM_STREAM_CAPTURE]; > > - if (psubstream) { > - err = snd_pcm_oss_make_ready(psubstream); > - if (err < 0) > - return err; > - } > - if (csubstream) { > - err = snd_pcm_oss_make_ready(csubstream); > - if (err < 0) > - return err; > - } > if (psubstream) { > runtime = psubstream->runtime; > cmd = 0; > if (mutex_lock_interruptible(&runtime->oss.params_lock)) > return -ERESTARTSYS; > + err = snd_pcm_oss_make_ready_locked(psubstream); > + if (err < 0) > + goto _skip1; > if (trigger & PCM_ENABLE_OUTPUT) { > if (runtime->oss.trigger) > goto _skip1; > @@ -2128,6 +2121,9 @@ static int snd_pcm_oss_set_trigger(struct snd_pcm_oss_file *pcm_oss_file, int tr > cmd = 0; > if (mutex_lock_interruptible(&runtime->oss.params_lock)) > return -ERESTARTSYS; > + err = snd_pcm_oss_make_ready_locked(csubstream); > + if (err < 0) > + goto _skip2; > if (trigger & PCM_ENABLE_INPUT) { > if (runtime->oss.trigger) > goto _skip2; > -- > 2.37.2 >