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 394CFE7D0AB for ; Thu, 21 Sep 2023 22:25:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232336AbjIUWZs (ORCPT ); Thu, 21 Sep 2023 18:25:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59780 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232873AbjIUWZ2 (ORCPT ); Thu, 21 Sep 2023 18:25:28 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9A61312451 for ; Thu, 21 Sep 2023 10:06:54 -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 05151338AA; Thu, 21 Sep 2023 13:29:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1695302968; 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=B4ukeL06gNkW2qwY73xzCc5or7ZOWYu47UFNiHA4sRE=; b=uJQgO2DmrOlob0nOo/gvqcHgDMSNFGUM2gGXPdp79ntAOsVO7hmqxktnsfDHPFZniVf01T v7Kn5AgQT6huYctuoqJ8LwO6G7g0cwZ44V3ggVgYfN2VUsSeIQQ80bnmTP05396MlpEmII fSA/dV5eo7ZFdyCoaSK4+dAcLUSAbRg= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1695302968; 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=B4ukeL06gNkW2qwY73xzCc5or7ZOWYu47UFNiHA4sRE=; b=ieGH00UspKgSKl5QEaY/iWTETyiW7Scz3JDoW1tgNAc/gFtBfLNVWlYo33/zUyHcj0IeaR NPwDU9XbPWCYX2Ag== 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 BA60B134B0; Thu, 21 Sep 2023 13:29:27 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id A5qWLDdFDGV0OQAAMHmgww (envelope-from ); Thu, 21 Sep 2023 13:29:27 +0000 Date: Thu, 21 Sep 2023 15:29:27 +0200 Message-ID: <87il83zmbs.wl-tiwai@suse.de> From: Takashi Iwai To: Ma Ke Cc: perex@perex.cz, tiwai@suse.com, Liam.Howlett@Oracle.com, rppt@kernel.org, mgorman@techsingularity.net, mhocko@suse.com, surenb@google.com, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] ALSA: pcm: oss: Fix race at SNDCTL_DSP_SETTRIGGER In-Reply-To: <20230921064258.3582115-1-make_ruc2021@163.com> References: <20230921064258.3582115-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 Thu, 21 Sep 2023 08:42:58 +0200, Ma Ke wrote: > > 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. > 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 > --- > sound/core/oss/pcm_oss.c | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > > diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c > index 728c211142d1..f6340a2fe52b 100644 > --- a/sound/core/oss/pcm_oss.c > +++ b/sound/core/oss/pcm_oss.c > @@ -2083,21 +2083,15 @@ 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) > + mutex_unlock(&runtime->oss.params_lock); > + return err; This breaks totally; you missed braces... (Ditto for another place). Takashi > if (trigger & PCM_ENABLE_OUTPUT) { > if (runtime->oss.trigger) > goto _skip1; > @@ -2128,6 +2122,10 @@ 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) > + mutex_unlock(&runtime->oss.params_lock); > + return err; > if (trigger & PCM_ENABLE_INPUT) { > if (runtime->oss.trigger) > goto _skip2; > -- > 2.37.2 >