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 X-Spam-Level: X-Spam-Status: No, score=-11.7 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9CF06C38A2A for ; Thu, 7 May 2020 10:20:24 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 27EAB2075E for ; Thu, 7 May 2020 10:20:24 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=alsa-project.org header.i=@alsa-project.org header.b="lFusgr8y" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 27EAB2075E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 569F316FC; Thu, 7 May 2020 12:19:32 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 569F316FC DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1588846822; bh=DSUwVYCNZH7gzg+Gu5y90XpAt4SZATu066wxRyB5xuo=; h=Date:From:To:Subject:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=lFusgr8y8Ns3qKwkevtnMyPKdg78zFfIxMXuyxlKPbS/+Um4OzEegpRd+Iq0TwtIa z8PIO/LKvPMuOzbk1mJB7vNGzIzDIEF44Ulez8LyNdi5Akf5tpoUXXtUPVdfF6/NDd plsw8Em71cPohSYik2skrmIvrfNZHfa0qfi/9Es4= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id A8BC1F8015B; Thu, 7 May 2020 12:19:31 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 86AFEF8015F; Thu, 7 May 2020 12:19:26 +0200 (CEST) Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id A26E7F800AD for ; Thu, 7 May 2020 12:19:20 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz A26E7F800AD X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id CDAE8AF99; Thu, 7 May 2020 10:19:21 +0000 (UTC) Date: Thu, 07 May 2020 12:19:18 +0200 Message-ID: From: Takashi Iwai To: Greg Kroah-Hartman Subject: Re: KASAN: use-after-free Write in snd_rawmidi_kernel_write1 In-Reply-To: <20200507101310.GA1311017@kroah.com> References: <20200507082302.GF1024567@kroah.com> <20200507101310.GA1311017@kroah.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: text/plain; charset=US-ASCII Cc: security@kernel.org, alsa-devel@alsa-project.org, butt3rflyh4ck , tiwai@suse.com, linux-kernel@vger.kernel.org, syzkaller X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On Thu, 07 May 2020 12:13:10 +0200, Greg Kroah-Hartman wrote: > > On Thu, May 07, 2020 at 11:56:22AM +0200, Takashi Iwai wrote: > > On Thu, 07 May 2020 10:23:02 +0200, > > Greg Kroah-Hartman wrote: > > > > > > On Thu, May 07, 2020 at 04:04:25PM +0800, butt3rflyh4ck wrote: > > > > I report a bug (in linux-5.7-rc1) found by syzkaller. > > > > > > > > kernel config: https://github.com/butterflyhack/syzkaller-fuzz/blob/master/v5.7.0-rc1.config > > > > reproducer: https://github.com/butterflyhack/syzkaller-fuzz/blob/master/repro.cprog > > > > > > > > I test the reproducer in linux-5.7-rc4 and crash too. > > > > > > Great, care to create a fix for this and send it to the proper > > > maintainers? That's the best way to get it fixed, otherwise it just > > > goes in the file with the rest of the syzbot reports we are burried > > > under. > > > > Don't worry, I already prepared a fix patch below :) > > > > > > thanks, > > > > Takashi > > > > -- 8< -- > > From: Takashi Iwai > > Subject: [PATCH] ALSA: rawmidi: Fix racy buffer resize under concurrent > > accesses > > > > The rawmidi core allows user to resize the runtime buffer via ioctl, > > and this may lead to UAF when performed during concurrent reads or > > writes. > > > > This patch fixes the race by introducing a reference counter for the > > runtime buffer access and returns -EBUSY error when the resize is > > performed concurrently. > > > > Reported-by: butt3rflyh4ck > > Cc: > > Link: https://lore.kernel.org/r/CAFcO6XMWpUVK_yzzCpp8_XP7+=oUpQvuBeCbMffEDkpe8jWrfg@mail.gmail.com > > Signed-off-by: Takashi Iwai > > --- > > include/sound/rawmidi.h | 1 + > > sound/core/rawmidi.c | 29 ++++++++++++++++++++++++++++- > > 2 files changed, 29 insertions(+), 1 deletion(-) > > > > diff --git a/include/sound/rawmidi.h b/include/sound/rawmidi.h > > index a36b7227a15a..334842daa904 100644 > > --- a/include/sound/rawmidi.h > > +++ b/include/sound/rawmidi.h > > @@ -61,6 +61,7 @@ struct snd_rawmidi_runtime { > > size_t avail_min; /* min avail for wakeup */ > > size_t avail; /* max used buffer for wakeup */ > > size_t xruns; /* over/underruns counter */ > > + int buffer_ref; /* buffer reference count */ > > /* misc */ > > spinlock_t lock; > > wait_queue_head_t sleep; > > diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c > > index 20dd08e1f675..4185d9e81e3c 100644 > > --- a/sound/core/rawmidi.c > > +++ b/sound/core/rawmidi.c > > @@ -120,6 +120,17 @@ static void snd_rawmidi_input_event_work(struct work_struct *work) > > runtime->event(runtime->substream); > > } > > > > +/* buffer refcount management: call with runtime->lock held */ > > +static inline void snd_rawmidi_buffer_ref(struct snd_rawmidi_runtime *runtime) > > +{ > > + runtime->buffer_ref++; > > +} > > + > > +static inline void snd_rawmidi_buffer_unref(struct snd_rawmidi_runtime *runtime) > > +{ > > + runtime->buffer_ref--; > > +} > > Why not use the reference count structure? The context accessing the buffer is always with the spinlock, so we don't need expensive atomic ops there. Usually this kind of check can be a simple boolean flag, but in this case, there is one place that goes out of lock due to copy_from/to_user, so a refcount is used in this patch instead. thanks, Takashi 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 X-Spam-Level: X-Spam-Status: No, score=-11.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 20A1BC38A2A for ; Thu, 7 May 2020 10:19:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 03DCC2073A for ; Thu, 7 May 2020 10:19:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725948AbgEGKTZ (ORCPT ); Thu, 7 May 2020 06:19:25 -0400 Received: from mx2.suse.de ([195.135.220.15]:47652 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725809AbgEGKTV (ORCPT ); Thu, 7 May 2020 06:19:21 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id CDAE8AF99; Thu, 7 May 2020 10:19:21 +0000 (UTC) Date: Thu, 07 May 2020 12:19:18 +0200 Message-ID: From: Takashi Iwai To: Greg Kroah-Hartman Cc: butt3rflyh4ck , security@kernel.org, syzkaller , tiwai@suse.com, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org Subject: Re: KASAN: use-after-free Write in snd_rawmidi_kernel_write1 In-Reply-To: <20200507101310.GA1311017@kroah.com> References: <20200507082302.GF1024567@kroah.com> <20200507101310.GA1311017@kroah.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: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 07 May 2020 12:13:10 +0200, Greg Kroah-Hartman wrote: > > On Thu, May 07, 2020 at 11:56:22AM +0200, Takashi Iwai wrote: > > On Thu, 07 May 2020 10:23:02 +0200, > > Greg Kroah-Hartman wrote: > > > > > > On Thu, May 07, 2020 at 04:04:25PM +0800, butt3rflyh4ck wrote: > > > > I report a bug (in linux-5.7-rc1) found by syzkaller. > > > > > > > > kernel config: https://github.com/butterflyhack/syzkaller-fuzz/blob/master/v5.7.0-rc1.config > > > > reproducer: https://github.com/butterflyhack/syzkaller-fuzz/blob/master/repro.cprog > > > > > > > > I test the reproducer in linux-5.7-rc4 and crash too. > > > > > > Great, care to create a fix for this and send it to the proper > > > maintainers? That's the best way to get it fixed, otherwise it just > > > goes in the file with the rest of the syzbot reports we are burried > > > under. > > > > Don't worry, I already prepared a fix patch below :) > > > > > > thanks, > > > > Takashi > > > > -- 8< -- > > From: Takashi Iwai > > Subject: [PATCH] ALSA: rawmidi: Fix racy buffer resize under concurrent > > accesses > > > > The rawmidi core allows user to resize the runtime buffer via ioctl, > > and this may lead to UAF when performed during concurrent reads or > > writes. > > > > This patch fixes the race by introducing a reference counter for the > > runtime buffer access and returns -EBUSY error when the resize is > > performed concurrently. > > > > Reported-by: butt3rflyh4ck > > Cc: > > Link: https://lore.kernel.org/r/CAFcO6XMWpUVK_yzzCpp8_XP7+=oUpQvuBeCbMffEDkpe8jWrfg@mail.gmail.com > > Signed-off-by: Takashi Iwai > > --- > > include/sound/rawmidi.h | 1 + > > sound/core/rawmidi.c | 29 ++++++++++++++++++++++++++++- > > 2 files changed, 29 insertions(+), 1 deletion(-) > > > > diff --git a/include/sound/rawmidi.h b/include/sound/rawmidi.h > > index a36b7227a15a..334842daa904 100644 > > --- a/include/sound/rawmidi.h > > +++ b/include/sound/rawmidi.h > > @@ -61,6 +61,7 @@ struct snd_rawmidi_runtime { > > size_t avail_min; /* min avail for wakeup */ > > size_t avail; /* max used buffer for wakeup */ > > size_t xruns; /* over/underruns counter */ > > + int buffer_ref; /* buffer reference count */ > > /* misc */ > > spinlock_t lock; > > wait_queue_head_t sleep; > > diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c > > index 20dd08e1f675..4185d9e81e3c 100644 > > --- a/sound/core/rawmidi.c > > +++ b/sound/core/rawmidi.c > > @@ -120,6 +120,17 @@ static void snd_rawmidi_input_event_work(struct work_struct *work) > > runtime->event(runtime->substream); > > } > > > > +/* buffer refcount management: call with runtime->lock held */ > > +static inline void snd_rawmidi_buffer_ref(struct snd_rawmidi_runtime *runtime) > > +{ > > + runtime->buffer_ref++; > > +} > > + > > +static inline void snd_rawmidi_buffer_unref(struct snd_rawmidi_runtime *runtime) > > +{ > > + runtime->buffer_ref--; > > +} > > Why not use the reference count structure? The context accessing the buffer is always with the spinlock, so we don't need expensive atomic ops there. Usually this kind of check can be a simple boolean flag, but in this case, there is one place that goes out of lock due to copy_from/to_user, so a refcount is used in this patch instead. thanks, Takashi