diff mbox series

[v2,2/2] ALSA: fireface: remove unnecessary condition in hwdep_read()

Message ID YA6n6I8EcNAO5ZFs@mwanda (mailing list archive)
State New, archived
Headers show
Series [v2,1/2] ALSA: oxfw: remove an unnecessary condition in hwdep_read() | expand

Commit Message

Dan Carpenter Jan. 25, 2021, 11:13 a.m. UTC
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(-)

Comments

Takashi Sakamoto Jan. 25, 2021, 1:45 p.m. UTC | #1
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
Takashi Iwai Jan. 25, 2021, 1:51 p.m. UTC | #2
On Mon, 25 Jan 2021 12:13:44 +0100,
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

Applied now.  Thanks.


Takashi
diff mbox series

Patch

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);