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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id E9E72C433EF for ; Wed, 6 Jul 2022 08:55:57 +0000 (UTC) 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 077181672; Wed, 6 Jul 2022 10:55:06 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 077181672 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1657097756; bh=U0htDIIwhpcah+EPk3c0bva5ZhF6BmA8fSZxA04XU+o=; h=Date:Subject:To:References:From:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=MqL+MLeldGMiS7lyn738T8/4Nuzn3ymkywdUXZDVNCrjiC8gWixrOaug2geJOkRHj UOFJiaGySBPZRE654XQHBxjrXcMIvO1g32gcj5hu6LTMXETFwSvmLIftmumFhHb3wk uNdIjVDsKsT05IuVh6BMOxCUnxz7YGy+8aaWrBiw= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 8F8FEF804B4; Wed, 6 Jul 2022 10:55:04 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id F0D1BF8023A; Wed, 6 Jul 2022 10:55:01 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 16396F8012A; Wed, 6 Jul 2022 10:54:51 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 16396F8012A Authentication-Results: alsa1.perex.cz; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="ZVM5jCRC" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1657097698; x=1688633698; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=U0htDIIwhpcah+EPk3c0bva5ZhF6BmA8fSZxA04XU+o=; b=ZVM5jCRC7C2ylpw52MAVhen8f9UHHe+XXlNxqHRUVYNwLN7Wsxavaeqa 6NABhUPjLn3IjPuDo7Ou5ytRKySg35dhPgcw62w3w2zEOIQm/ZSjlQ6On 1mxildUHNuYtAqg7lQRWlgsYNWuHZFg47mxuZBXG/xi2pQO4gHSYj+9Ju 6kKk5/Sr63BprSprlc5jvSVLr9Tj49EC/zkXYzTHa2Xvv5g69k1W50jeG DT3ky/8nR/bg6dTALRKvO49sw5atriqGnB9Uuyyh9sy0S8GhaBVR2Wu0X vpfjg18EEiDk8FgSYEoRuoXw36Uh428Vf+31DB+NcNCvs1iEvoSSKHN3Z g==; X-IronPort-AV: E=McAfee;i="6400,9594,10399"; a="370005141" X-IronPort-AV: E=Sophos;i="5.92,249,1650956400"; d="scan'208";a="370005141" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Jul 2022 01:54:49 -0700 X-IronPort-AV: E=Sophos;i="5.92,249,1650956400"; d="scan'208";a="920075962" Received: from gguerra-mobl1.ger.corp.intel.com (HELO [10.249.254.46]) ([10.249.254.46]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Jul 2022 01:54:46 -0700 Message-ID: <4cf393e1-00f6-2b5b-a5f5-9f555fdeafdc@linux.intel.com> Date: Wed, 6 Jul 2022 11:55:33 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Firefox/91.0 Thunderbird/91.10.0 Subject: Re: [PATCH] ASoC: SOF: ipc-msg-injector: fix copy in sof_msg_inject_ipc4_dfs_write() Content-Language: en-US To: Dan Carpenter , Pierre-Louis Bossart References: From: =?UTF-8?Q?P=c3=a9ter_Ujfalusi?= In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: alsa-devel@alsa-project.org, Kai Vehmanen , Daniel Baluta , kernel-janitors@vger.kernel.org, Takashi Iwai , Liam Girdwood , Mark Brown , Ranjani Sridharan , Bard Liao , sound-open-firmware@alsa-project.org 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 06/07/2022 10:01, Dan Carpenter wrote: > The bug here is that when we copy the payload the value of *ppos should > be zero but it is sizeof(ipc4_msg->header_u64) instead. That means that > the buffer will be copied to the wrong location within the > ipc4_msg->data_ptr buffer. > > Really, in this context, it is simpler and more appropriate to use > copy_from_user() instead of simple_write_to_buffer() so I have > re-written the function. > > Fixes: 066c67624d8c ("ASoC: SOF: ipc-msg-injector: Add support for IPC4 messages") > Signed-off-by: Dan Carpenter > --- > From static analysis. Not tested. I believe this function is mostly > used to write random junk to the device and see what breaks. So > probably it works fine as-is. > > sound/soc/sof/sof-client-ipc-msg-injector.c | 27 ++++++++------------- > 1 file changed, 10 insertions(+), 17 deletions(-) > > diff --git a/sound/soc/sof/sof-client-ipc-msg-injector.c b/sound/soc/sof/sof-client-ipc-msg-injector.c > index 6bdfa527b7f7..e8ab173e95b5 100644 > --- a/sound/soc/sof/sof-client-ipc-msg-injector.c > +++ b/sound/soc/sof/sof-client-ipc-msg-injector.c > @@ -181,7 +181,7 @@ static ssize_t sof_msg_inject_ipc4_dfs_write(struct file *file, > struct sof_client_dev *cdev = file->private_data; > struct sof_msg_inject_priv *priv = cdev->data; > struct sof_ipc4_msg *ipc4_msg = priv->tx_buffer; > - ssize_t size; > + size_t data_size; > int ret; > > if (*ppos) > @@ -191,25 +191,18 @@ static ssize_t sof_msg_inject_ipc4_dfs_write(struct file *file, > return -EINVAL; > > /* copy the header first */ > - size = simple_write_to_buffer(&ipc4_msg->header_u64, > - sizeof(ipc4_msg->header_u64), > - ppos, buffer, count); > - if (size < 0) > - return size; > - if (size != sizeof(ipc4_msg->header_u64)) > + if (copy_from_user(&ipc4_msg->header_u64, buffer, > + sizeof(ipc4_msg->header_u64))) > return -EFAULT; > > - count -= size; > + data_size = count - sizeof(ipc4_msg->header_u64); > + if (data_size > priv->max_msg_size) > + return -EINVAL; > /* Copy the payload */ > - size = simple_write_to_buffer(ipc4_msg->data_ptr, > - priv->max_msg_size, ppos, buffer, > - count); > - if (size < 0) > - return size; > - if (size != count) > + if (copy_from_user(ipc4_msg->data_ptr, buffer, data_size)) I think this is still needs an update: if (copy_from_user(ipc4_msg->data_ptr, buffer + sizeof(ipc4_msg->header_u64), data_size)) To skip the already copied header. Or without a rewrite the fix would be simple as: /* Copy the payload */ size = simple_write_to_buffer(ipc4_msg->data_ptr, 0, buffer + sizeof(ipc4_msg->header_u64), data_size), count); > return -EFAULT; > > - ipc4_msg->data_size = count; > + ipc4_msg->data_size = data_size; > > /* Initialize the reply storage */ > ipc4_msg = priv->rx_buffer; > @@ -221,9 +214,9 @@ static ssize_t sof_msg_inject_ipc4_dfs_write(struct file *file, > > /* return the error code if test failed */ > if (ret < 0) > - size = ret; > + count = ret; > > - return size; > + return count; > }; > > static int sof_msg_inject_dfs_release(struct inode *inode, struct file *file) -- Péter