From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: alsa-devel@alsa-project.org, kernel-janitors@vger.kernel.org,
Clemens Ladisch <clemens@ladisch.de>,
Takashi Iwai <tiwai@suse.com>, Mark Brown <broonie@kernel.org>,
Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Subject: Re: [PATCH v2 2/2] ALSA: fireface: remove unnecessary condition in hwdep_read()
Date: Mon, 25 Jan 2021 22:45:38 +0900 [thread overview]
Message-ID: <20210125134538.GA24062@workstation> (raw)
In-Reply-To: <YA6n6I8EcNAO5ZFs@mwanda>
Hi,
On Mon, Jan 25, 2021 at 02:13:44PM +0300, Dan Carpenter wrote:
> Smatch complains that "count" is not clamped when "ff->dev_lock_changed"
> and it leads to an information leak. Fortunately, that's not actually
> possible and the condition can be deleted.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: just delet the condition
>
> sound/firewire/fireface/ff-hwdep.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
Acked-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
ALSA firewire stack includes some drivers. Although some of them implement
unique event as well as the lock event, the others supports the lock
event only. The condition statement comes from the former, I guess.
ALSA BeBoB, OXFW, and Fireface drivers are the latter. Later I'll post
the similar patch for ALSA BeBoB driver.
Anyway, thank you to find the issue ;)
> diff --git a/sound/firewire/fireface/ff-hwdep.c b/sound/firewire/fireface/ff-hwdep.c
> index 4b2e0dff5ddb..ea64a2a41eea 100644
> --- a/sound/firewire/fireface/ff-hwdep.c
> +++ b/sound/firewire/fireface/ff-hwdep.c
> @@ -35,13 +35,11 @@ static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf, long count,
> }
>
> memset(&event, 0, sizeof(event));
> - if (ff->dev_lock_changed) {
> - event.lock_status.type = SNDRV_FIREWIRE_EVENT_LOCK_STATUS;
> - event.lock_status.status = (ff->dev_lock_count > 0);
> - ff->dev_lock_changed = false;
> + event.lock_status.type = SNDRV_FIREWIRE_EVENT_LOCK_STATUS;
> + event.lock_status.status = (ff->dev_lock_count > 0);
> + ff->dev_lock_changed = false;
>
> - count = min_t(long, count, sizeof(event.lock_status));
> - }
> + count = min_t(long, count, sizeof(event.lock_status));
>
> spin_unlock_irq(&ff->lock);
>
> --
> 2.29.2
Thanks
Takashi Sakamoto
next prev parent reply other threads:[~2021-01-25 13:46 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-21 6:10 [PATCH] ALSA: fireface: fix info leak in hwdep_read() Dan Carpenter
2021-01-21 20:42 ` Christophe JAILLET
2021-01-22 7:13 ` Dan Carpenter
2021-01-25 11:12 ` [PATCH v2 1/2] ALSA: oxfw: remove an unnecessary condition " Dan Carpenter
2021-01-25 13:46 ` Takashi Sakamoto
2021-01-25 13:51 ` Takashi Iwai
2021-01-25 11:13 ` [PATCH v2 2/2] ALSA: fireface: remove " Dan Carpenter
2021-01-25 13:45 ` Takashi Sakamoto [this message]
2021-01-25 13:51 ` Takashi Iwai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210125134538.GA24062@workstation \
--to=o-takashi@sakamocchi.jp \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=christophe.jaillet@wanadoo.fr \
--cc=clemens@ladisch.de \
--cc=dan.carpenter@oracle.com \
--cc=kernel-janitors@vger.kernel.org \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).