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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8C0C3C433EF for ; Wed, 20 Oct 2021 05:41:52 +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 378B360E8B for ; Wed, 20 Oct 2021 05:41:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 378B360E8B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=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 4CA611695; Wed, 20 Oct 2021 07:40:58 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 4CA611695 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1634708508; bh=j7sxik9qd7/ZgNRvUOm2axWv4n5rUL98k6DxX6MQxLs=; h=Date:From:To:Subject:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=QhP2HTvKOciTt4+/XCR8Xrp1NwcBRVaZIo/aoFucvZx5S/koWYqUEf8nTvhTlrqpB T1xfHIFTU/Azv+en0N8saesW5sDMnAc7VJ6MHYs3kw8hKpjpb1Zf0NfWb3kkUC8/4g yuOeQfB/tOhdfao0L4Dxs1WZKqldm5k15f5BvXVA= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id B7347F80224; Wed, 20 Oct 2021 07:40:57 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 0DB52F80229; Wed, 20 Oct 2021 07:40:56 +0200 (CEST) Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 4CC9EF800ED for ; Wed, 20 Oct 2021 07:40:48 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 4CC9EF800ED Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="ZSmSk+kA"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="+XUugQmG" Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 13EAE21A7F; Wed, 20 Oct 2021 05:40:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1634708448; 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=uTikBjU/lH6hfwNd7SXAfhv6iSZLQ8kHu/v5yDzs6nU=; b=ZSmSk+kANpbD+QPYYOC01SEKh5b+psZuT/YWk5l/FMPKxEf3CgR8uAzqcYLZSGkBflS96c IwCkafqPaTVZ7TN+tI+DNsm/qXyEb8uOYJpXy7VY5KOcJwBW6S0fdfxYu/mhO9K0lzdH+B AgMRG3EIvipdQ82Qa1AO84oHjK9kPXk= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1634708448; 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=uTikBjU/lH6hfwNd7SXAfhv6iSZLQ8kHu/v5yDzs6nU=; b=+XUugQmGEQ1pVAaUXYaCT6RVmIPT4DfzGjuyLthnxQLOz8laWXazrKOshP0Qv5vOgTk6ix TF3b1o9ObF8lxjAA== Received: from alsa1.suse.de (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id DA215A3B81; Wed, 20 Oct 2021 05:40:46 +0000 (UTC) Date: Wed, 20 Oct 2021 07:40:46 +0200 Message-ID: From: Takashi Iwai To: Takashi Sakamoto Subject: Re: [PATCH] ALSA: firewire-motu: fix invalid memory access when operating hwdep character device In-Reply-To: <20211020042555.40866-1-o-takashi@sakamocchi.jp> References: <20211020042555.40866-1-o-takashi@sakamocchi.jp> 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: alsa-devel@alsa-project.org, clemens@ladisch.de 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 Wed, 20 Oct 2021 06:25:55 +0200, Takashi Sakamoto wrote: > > ALSA firewire-motu driver recently got support for event notification via > ALSA HwDep interface for register DSP models. However, when polling ALSA > HwDep cdev, the driver can cause null pointer dereference for the other > models due to accessing to unallocated memory or uninitialized memory. > > This commit fixes the bug by check the type of model before accessing to > the memory. > > Fixes: 634ec0b2906e ("ALSA: firewire-motu: notify event for parameter change in register DSP model") > Signed-off-by: Takashi Sakamoto Wouldn't it be simpler to add the flag check in snd_motu_register_dsp_message_parser_count_event() and return 0 if SND_MOTU_SPEC_REGISTER_DSP isn't set there? thanks, Takashi > --- > sound/firewire/motu/motu-hwdep.c | 72 ++++++++++++++++++++------------ > 1 file changed, 45 insertions(+), 27 deletions(-) > > diff --git a/sound/firewire/motu/motu-hwdep.c b/sound/firewire/motu/motu-hwdep.c > index 9c2e457ce692..ae2d01ddc8d3 100644 > --- a/sound/firewire/motu/motu-hwdep.c > +++ b/sound/firewire/motu/motu-hwdep.c > @@ -16,6 +16,47 @@ > > #include "motu.h" > > +static bool has_dsp_event(struct snd_motu *motu) > +{ > + if (motu->spec->flags & SND_MOTU_SPEC_REGISTER_DSP) > + return (snd_motu_register_dsp_message_parser_count_event(motu) > 0); > + else > + return false; > +} > + > +// NOTE: Take care of page fault due to accessing to userspace. > +static long copy_dsp_event_to_user(struct snd_motu *motu, char __user *buf, long count, > + struct snd_firewire_event_motu_register_dsp_change *event) > +{ > + if (motu->spec->flags & SND_MOTU_SPEC_REGISTER_DSP) { > + size_t consumed = 0; > + u32 __user *ptr; > + u32 ev; > + > + // Header is filled later. > + consumed += sizeof(*event); > + > + while (consumed < count && > + snd_motu_register_dsp_message_parser_copy_event(motu, &ev)) { > + ptr = (u32 __user *)(buf + consumed); > + if (put_user(ev, ptr)) > + return -EFAULT; > + consumed += sizeof(ev); > + } > + > + event->type = SNDRV_FIREWIRE_EVENT_MOTU_REGISTER_DSP_CHANGE; > + event->count = (consumed - sizeof(*event)) / 4; > + if (copy_to_user(buf, &event, sizeof(*event))) > + return -EFAULT; > + > + count = consumed; > + } else { > + count = 0; > + } > + > + return count; > +} > + > static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf, long count, > loff_t *offset) > { > @@ -25,8 +66,7 @@ static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf, long count, > > spin_lock_irq(&motu->lock); > > - while (!motu->dev_lock_changed && motu->msg == 0 && > - snd_motu_register_dsp_message_parser_count_event(motu) == 0) { > + while (!motu->dev_lock_changed && motu->msg == 0 && !has_dsp_event(motu)) { > prepare_to_wait(&motu->hwdep_wait, &wait, TASK_INTERRUPTIBLE); > spin_unlock_irq(&motu->lock); > schedule(); > @@ -55,31 +95,10 @@ static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf, long count, > count = min_t(long, count, sizeof(event)); > if (copy_to_user(buf, &event, count)) > return -EFAULT; > - } else if (snd_motu_register_dsp_message_parser_count_event(motu) > 0) { > - size_t consumed = 0; > - u32 __user *ptr; > - u32 ev; > - > + } else if (has_dsp_event(motu)) { > spin_unlock_irq(&motu->lock); > > - // Header is filled later. > - consumed += sizeof(event.motu_register_dsp_change); > - > - while (consumed < count && > - snd_motu_register_dsp_message_parser_copy_event(motu, &ev)) { > - ptr = (u32 __user *)(buf + consumed); > - if (put_user(ev, ptr)) > - return -EFAULT; > - consumed += sizeof(ev); > - } > - > - event.motu_register_dsp_change.type = SNDRV_FIREWIRE_EVENT_MOTU_REGISTER_DSP_CHANGE; > - event.motu_register_dsp_change.count = > - (consumed - sizeof(event.motu_register_dsp_change)) / 4; > - if (copy_to_user(buf, &event, sizeof(event.motu_register_dsp_change))) > - return -EFAULT; > - > - count = consumed; > + count = copy_dsp_event_to_user(motu, buf, count, &event.motu_register_dsp_change); > } > > return count; > @@ -94,8 +113,7 @@ static __poll_t hwdep_poll(struct snd_hwdep *hwdep, struct file *file, > poll_wait(file, &motu->hwdep_wait, wait); > > spin_lock_irq(&motu->lock); > - if (motu->dev_lock_changed || motu->msg || > - snd_motu_register_dsp_message_parser_count_event(motu) > 0) > + if (motu->dev_lock_changed || motu->msg || has_dsp_event(motu)) > events = EPOLLIN | EPOLLRDNORM; > else > events = 0; > -- > 2.30.2 >