From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752320AbeADRDG (ORCPT + 1 other); Thu, 4 Jan 2018 12:03:06 -0500 Received: from mx2.suse.de ([195.135.220.15]:55832 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751441AbeADRDE (ORCPT ); Thu, 4 Jan 2018 12:03:04 -0500 Date: Thu, 04 Jan 2018 18:03:01 +0100 Message-ID: From: Takashi Iwai To: Dmitry Vyukov Cc: alsa-devel@alsa-project.org, Jaroslav Kysela , syzbot , =?UTF-8?B?RnI=?= =?UTF-8?B?w6lkw6lyaWM=?= Weisbecker , syzkaller-bugs@googlegroups.com, Ingo Molnar , Thomas Gleixner , LKML Subject: Re: INFO: rcu detected stall in memcpy In-Reply-To: References: <001a113f711a23fe310561f21d1a@google.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: multipart/mixed; boundary="Multipart_Thu_Jan__4_18:03:01_2018-1" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: --Multipart_Thu_Jan__4_18:03:01_2018-1 Content-Type: text/plain; charset=US-ASCII On Thu, 04 Jan 2018 15:17:23 +0100, Takashi Iwai wrote: > > On Thu, 04 Jan 2018 15:01:06 +0100, > Dmitry Vyukov wrote: > > > > On Thu, Jan 4, 2018 at 1:57 PM, Takashi Iwai wrote: > > > On Thu, 04 Jan 2018 13:08:45 +0100, > > > Dmitry Vyukov wrote: > > >> > > >> On Thu, Jan 4, 2018 at 1:03 PM, syzbot > > >> wrote: > > >> > Hello, > > >> > > > >> > syzkaller hit the following crash on > > >> > 30a7acd573899fd8b8ac39236eff6468b195ac7d > > >> > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master > > >> > compiler: gcc (GCC) 7.1.1 20170620 > > >> > .config is attached > > >> > Raw console output is attached. > > >> > Unfortunately, I don't have any reproducer for this bug yet. > > >> > > > >> > > > >> > IMPORTANT: if you fix the bug, please add the following tag to the commit: > > >> > Reported-by: syzbot+387f48da65cb522abfe8@syzkaller.appspotmail.com > > >> > It will help syzbot understand when the bug is fixed. See footer for > > >> > details. > > >> > If you forward the report, please keep this part and the footer. > > >> > > >> This looks ALSA-related. +ALSA maintainers. > > > > > > Not sure exactly what triggers it. It's the simple memcpy(), and I > > > don't know where RCU is involved in that code path. > > > > > > BTW, other two suspicious RCU usage reports are actually stopped at > > > the second WARN_ON() after the RCU message, and the second WARN_ON() > > > is independent from RCU; it's the known spurious WARN_ON() and was > > > already removed in the sound git tree. > > > > > > Hi Takashi, > > > > Another similar one just popped up: > > > > https://groups.google.com/forum/#!topic/syzkaller-bugs/X3d6-PIrJM0 > > > > This looks like mulaw_decode enters an infinite loop, or at least > > doing very large amount of computations without a resched, e.g. > > (uint64_t)-1 number of iterations of something along these lines. > > OK, that makes sense. > > My rough guess is that it's the misconfigured aloop device by > concurrent setup. The aloop device allows to restrict the parameters > of the other side of the connection, and something bad may happen > there if both sides are updated concurrently. > > We've seen segfault by memset() at loopback_preapre() in > sound/drivers/aloop.c by syzbot+3902b5220e8ca27889ca, too, which > indicates also the wrongly setup parameters that overflows the > allocated buffer. Below two patches may possibly plug the holes, but I'm not entirely sure whether that's the exact culprit. Could you put them into syzbot to watch whether they have any influence? In anyway, they are obvious bugs to be fixed, so I'm going to queue to my tree. thanks, Takashi --Multipart_Thu_Jan__4_18:03:01_2018-1 Content-Type: application/octet-stream; type=patch Content-Disposition: attachment; filename="0001-ALSA-pcm-Add-missing-error-checks-in-OSS-emulation-p.patch" Content-Transfer-Encoding: 7bit >>From 6708913750344a900f2e73bfe4a4d6dbbce4fe8d Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 4 Jan 2018 16:39:27 +0100 Subject: [PATCH 1/2] ALSA: pcm: Add missing error checks in OSS emulation plugin builder In the OSS emulation plugin builder where the frame size is parsed in the plugin chain, some places miss the possible errors returned from the plugin src_ or dst_frames callback. This patch papers over such places. Cc: Signed-off-by: Takashi Iwai --- sound/core/oss/pcm_plugin.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/sound/core/oss/pcm_plugin.c b/sound/core/oss/pcm_plugin.c index cadc93792868..85a56af104bd 100644 --- a/sound/core/oss/pcm_plugin.c +++ b/sound/core/oss/pcm_plugin.c @@ -592,18 +592,26 @@ snd_pcm_sframes_t snd_pcm_plug_write_transfer(struct snd_pcm_substream *plug, st snd_pcm_sframes_t frames = size; plugin = snd_pcm_plug_first(plug); - while (plugin && frames > 0) { + while (plugin) { + if (frames <= 0) + return frames; if ((next = plugin->next) != NULL) { snd_pcm_sframes_t frames1 = frames; - if (plugin->dst_frames) + if (plugin->dst_frames) { frames1 = plugin->dst_frames(plugin, frames); + if (frames1 <= 0) + return frames1; + } if ((err = next->client_channels(next, frames1, &dst_channels)) < 0) { return err; } if (err != frames1) { frames = err; - if (plugin->src_frames) + if (plugin->src_frames) { frames = plugin->src_frames(plugin, frames1); + if (frames <= 0) + return frames; + } } } else dst_channels = NULL; -- 2.15.1 --Multipart_Thu_Jan__4_18:03:01_2018-1 Content-Type: application/octet-stream; type=patch Content-Disposition: attachment; filename="0002-ALSA-aloop-Fix-racy-hw-constraints-adjustment.patch" Content-Transfer-Encoding: 7bit >>From ba043d316c911c80ec067bccff4e76e7c11619d3 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 4 Jan 2018 17:38:54 +0100 Subject: [PATCH 2/2] ALSA: aloop: Fix racy hw constraints adjustment The aloop driver tries to update the hw constraints of the connected target on the cable of the opened PCM substream. This is done by adding the extra hw constraints rules referring to the substream runtime->hw fields, while the other substream may update the runtime hw of another side on the fly. This is, however, racy and may result in the inconsistent values when both PCM streams perform the prepare concurrently. One of the reason is that it overwrites the other's runtime->hw field; which is not only racy but also broken when it's called before the open of another side finishes. And, since the reference to runtime->hw isn't protected, the concurrent write may give the partial value update and become inconsistent. This patch is an attempt to fix and clean up: - The prepare doesn't change the runtime->hw of other side any longer, but only update the cable->hw that is referred commonly. - The extra rules refer to the loopback_pcm object instead of the runtime->hw. The actual hw is deduced from cable->hw. - The extra rules take the cable_lock to protect against the race. Fixes: b1c73fc8e697 ("ALSA: snd-aloop: Fix hw_params restrictions and checking") Cc: Signed-off-by: Takashi Iwai --- sound/drivers/aloop.c | 51 +++++++++++++++++++++------------------------------ 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/sound/drivers/aloop.c b/sound/drivers/aloop.c index afac886ffa28..b1eee1a1e925 100644 --- a/sound/drivers/aloop.c +++ b/sound/drivers/aloop.c @@ -305,19 +305,6 @@ static int loopback_trigger(struct snd_pcm_substream *substream, int cmd) return 0; } -static void params_change_substream(struct loopback_pcm *dpcm, - struct snd_pcm_runtime *runtime) -{ - struct snd_pcm_runtime *dst_runtime; - - if (dpcm == NULL || dpcm->substream == NULL) - return; - dst_runtime = dpcm->substream->runtime; - if (dst_runtime == NULL) - return; - dst_runtime->hw = dpcm->cable->hw; -} - static void params_change(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime; @@ -329,10 +316,6 @@ static void params_change(struct snd_pcm_substream *substream) cable->hw.rate_max = runtime->rate; cable->hw.channels_min = runtime->channels; cable->hw.channels_max = runtime->channels; - params_change_substream(cable->streams[SNDRV_PCM_STREAM_PLAYBACK], - runtime); - params_change_substream(cable->streams[SNDRV_PCM_STREAM_CAPTURE], - runtime); } static int loopback_prepare(struct snd_pcm_substream *substream) @@ -620,12 +603,14 @@ static unsigned int get_cable_index(struct snd_pcm_substream *substream) static int rule_format(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule) { - - struct snd_pcm_hardware *hw = rule->private; + struct loopback_pcm *dpcm = rule->private; + struct loopback_cable *cable = dpcm->cable; struct snd_mask *maskp = hw_param_mask(params, rule->var); - maskp->bits[0] &= (u_int32_t)hw->formats; - maskp->bits[1] &= (u_int32_t)(hw->formats >> 32); + mutex_lock(&dpcm->loopback->cable_lock); + maskp->bits[0] &= (u_int32_t)cable->hw.formats; + maskp->bits[1] &= (u_int32_t)(cable->hw.formats >> 32); + mutex_unlock(&dpcm->loopback->cable_lock); memset(maskp->bits + 2, 0, (SNDRV_MASK_MAX-64) / 8); /* clear rest */ if (! maskp->bits[0] && ! maskp->bits[1]) return -EINVAL; @@ -635,11 +620,14 @@ static int rule_format(struct snd_pcm_hw_params *params, static int rule_rate(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule) { - struct snd_pcm_hardware *hw = rule->private; + struct loopback_pcm *dpcm = rule->private; + struct loopback_cable *cable = dpcm->cable; struct snd_interval t; - t.min = hw->rate_min; - t.max = hw->rate_max; + mutex_lock(&dpcm->loopback->cable_lock); + t.min = cable->hw.rate_min; + t.max = cable->hw.rate_max; + mutex_unlock(&dpcm->loopback->cable_lock); t.openmin = t.openmax = 0; t.integer = 0; return snd_interval_refine(hw_param_interval(params, rule->var), &t); @@ -648,11 +636,14 @@ static int rule_rate(struct snd_pcm_hw_params *params, static int rule_channels(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule) { - struct snd_pcm_hardware *hw = rule->private; + struct loopback_pcm *dpcm = rule->private; + struct loopback_cable *cable = dpcm->cable; struct snd_interval t; - t.min = hw->channels_min; - t.max = hw->channels_max; + mutex_lock(&dpcm->loopback->cable_lock); + t.min = cable->hw.channels_min; + t.max = cable->hw.channels_max; + mutex_unlock(&dpcm->loopback->cable_lock); t.openmin = t.openmax = 0; t.integer = 0; return snd_interval_refine(hw_param_interval(params, rule->var), &t); @@ -699,19 +690,19 @@ static int loopback_open(struct snd_pcm_substream *substream) /* are cached -> they do not reflect the actual state */ err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_FORMAT, - rule_format, &runtime->hw, + rule_format, dpcm, SNDRV_PCM_HW_PARAM_FORMAT, -1); if (err < 0) goto unlock; err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, - rule_rate, &runtime->hw, + rule_rate, dpcm, SNDRV_PCM_HW_PARAM_RATE, -1); if (err < 0) goto unlock; err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, - rule_channels, &runtime->hw, + rule_channels, dpcm, SNDRV_PCM_HW_PARAM_CHANNELS, -1); if (err < 0) goto unlock; -- 2.15.1 --Multipart_Thu_Jan__4_18:03:01_2018-1--