All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ALSA: fireface: fix info leak in hwdep_read()
@ 2021-01-21  6:10 ` Dan Carpenter
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2021-01-21  6:10 UTC (permalink / raw)
  To: Clemens Ladisch; +Cc: alsa-devel, kernel-janitors, Takashi Iwai, Mark Brown

If "ff->dev_lock_changed" has not changed and "count" is too large then
this will copy data beyond the end of the struct to user space.

Fixes: f656edd5fb33 ("ALSA: fireface: add hwdep interface")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 sound/firewire/fireface/ff-hwdep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/firewire/fireface/ff-hwdep.c b/sound/firewire/fireface/ff-hwdep.c
index 4b2e0dff5ddb..b84dde609a03 100644
--- a/sound/firewire/fireface/ff-hwdep.c
+++ b/sound/firewire/fireface/ff-hwdep.c
@@ -35,12 +35,12 @@ static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf,  long count,
 	}
 
 	memset(&event, 0, sizeof(event));
+	count = min_t(long, count, sizeof(event.lock_status));
 	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;
 
-		count = min_t(long, count, sizeof(event.lock_status));
 	}
 
 	spin_unlock_irq(&ff->lock);
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH] ALSA: fireface: fix info leak in hwdep_read()
@ 2021-01-21  6:10 ` Dan Carpenter
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2021-01-21  6:10 UTC (permalink / raw)
  To: Clemens Ladisch; +Cc: alsa-devel, kernel-janitors, Takashi Iwai, Mark Brown

If "ff->dev_lock_changed" has not changed and "count" is too large then
this will copy data beyond the end of the struct to user space.

Fixes: f656edd5fb33 ("ALSA: fireface: add hwdep interface")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 sound/firewire/fireface/ff-hwdep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/firewire/fireface/ff-hwdep.c b/sound/firewire/fireface/ff-hwdep.c
index 4b2e0dff5ddb..b84dde609a03 100644
--- a/sound/firewire/fireface/ff-hwdep.c
+++ b/sound/firewire/fireface/ff-hwdep.c
@@ -35,12 +35,12 @@ static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf,  long count,
 	}
 
 	memset(&event, 0, sizeof(event));
+	count = min_t(long, count, sizeof(event.lock_status));
 	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;
 
-		count = min_t(long, count, sizeof(event.lock_status));
 	}
 
 	spin_unlock_irq(&ff->lock);
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* Re: [PATCH] ALSA: fireface: fix info leak in hwdep_read()
  2021-01-21  6:10 ` Dan Carpenter
@ 2021-01-21 20:42   ` Christophe JAILLET
  -1 siblings, 0 replies; 18+ messages in thread
From: Christophe JAILLET @ 2021-01-21 20:42 UTC (permalink / raw)
  To: Dan Carpenter, Clemens Ladisch
  Cc: alsa-devel, Mark Brown, kernel-janitors, Takashi Iwai

Hi Dan,

Le 21/01/2021 à 07:10, Dan Carpenter a écrit :
> If "ff->dev_lock_changed" has not changed

According to the "while (!ff->dev_lock_changed) { ... }" just above and 
the lock in place, can this ever happen?

In other word, I wonder if the "if (ff->dev_lock_changed)" test makes 
sense and if it could be removed.


(same for your other patch against sound/firewire/oxfw/oxfw-hwdep.c)

CJ


> and "count" is too large then
> this will copy data beyond the end of the struct to user space.
> 
> Fixes: f656edd5fb33 ("ALSA: fireface: add hwdep interface")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>   sound/firewire/fireface/ff-hwdep.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/firewire/fireface/ff-hwdep.c b/sound/firewire/fireface/ff-hwdep.c
> index 4b2e0dff5ddb..b84dde609a03 100644
> --- a/sound/firewire/fireface/ff-hwdep.c
> +++ b/sound/firewire/fireface/ff-hwdep.c
> @@ -35,12 +35,12 @@ static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf,  long count,
>   	}
>   
>   	memset(&event, 0, sizeof(event));
> +	count = min_t(long, count, sizeof(event.lock_status));
>   	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;
>   
> -		count = min_t(long, count, sizeof(event.lock_status));
>   	}
>   
>   	spin_unlock_irq(&ff->lock);
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] ALSA: fireface: fix info leak in hwdep_read()
@ 2021-01-21 20:42   ` Christophe JAILLET
  0 siblings, 0 replies; 18+ messages in thread
From: Christophe JAILLET @ 2021-01-21 20:42 UTC (permalink / raw)
  To: Dan Carpenter, Clemens Ladisch
  Cc: alsa-devel, Mark Brown, kernel-janitors, Takashi Iwai

Hi Dan,

Le 21/01/2021 à 07:10, Dan Carpenter a écrit :
> If "ff->dev_lock_changed" has not changed

According to the "while (!ff->dev_lock_changed) { ... }" just above and 
the lock in place, can this ever happen?

In other word, I wonder if the "if (ff->dev_lock_changed)" test makes 
sense and if it could be removed.


(same for your other patch against sound/firewire/oxfw/oxfw-hwdep.c)

CJ


> and "count" is too large then
> this will copy data beyond the end of the struct to user space.
> 
> Fixes: f656edd5fb33 ("ALSA: fireface: add hwdep interface")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>   sound/firewire/fireface/ff-hwdep.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/firewire/fireface/ff-hwdep.c b/sound/firewire/fireface/ff-hwdep.c
> index 4b2e0dff5ddb..b84dde609a03 100644
> --- a/sound/firewire/fireface/ff-hwdep.c
> +++ b/sound/firewire/fireface/ff-hwdep.c
> @@ -35,12 +35,12 @@ static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf,  long count,
>   	}
>   
>   	memset(&event, 0, sizeof(event));
> +	count = min_t(long, count, sizeof(event.lock_status));
>   	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;
>   
> -		count = min_t(long, count, sizeof(event.lock_status));
>   	}
>   
>   	spin_unlock_irq(&ff->lock);
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] ALSA: fireface: fix info leak in hwdep_read()
  2021-01-21 20:42   ` Christophe JAILLET
@ 2021-01-22  7:13     ` Dan Carpenter
  -1 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2021-01-22  7:13 UTC (permalink / raw)
  To: Christophe JAILLET
  Cc: kernel-janitors, alsa-devel, Mark Brown, Clemens Ladisch, Takashi Iwai

On Thu, Jan 21, 2021 at 09:42:04PM +0100, Christophe JAILLET wrote:
> Hi Dan,
> 
> Le 21/01/2021 à 07:10, Dan Carpenter a écrit :
> > If "ff->dev_lock_changed" has not changed
> 
> According to the "while (!ff->dev_lock_changed) { ... }" just above and the
> lock in place, can this ever happen?
> 
> In other word, I wonder if the "if (ff->dev_lock_changed)" test makes sense
> and if it could be removed.
> 
> 
> (same for your other patch against sound/firewire/oxfw/oxfw-hwdep.c)
> 

Yeah.  That's a good point.  I'll resend.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH] ALSA: fireface: fix info leak in hwdep_read()
@ 2021-01-22  7:13     ` Dan Carpenter
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2021-01-22  7:13 UTC (permalink / raw)
  To: Christophe JAILLET
  Cc: kernel-janitors, alsa-devel, Mark Brown, Clemens Ladisch, Takashi Iwai

On Thu, Jan 21, 2021 at 09:42:04PM +0100, Christophe JAILLET wrote:
> Hi Dan,
> 
> Le 21/01/2021 à 07:10, Dan Carpenter a écrit :
> > If "ff->dev_lock_changed" has not changed
> 
> According to the "while (!ff->dev_lock_changed) { ... }" just above and the
> lock in place, can this ever happen?
> 
> In other word, I wonder if the "if (ff->dev_lock_changed)" test makes sense
> and if it could be removed.
> 
> 
> (same for your other patch against sound/firewire/oxfw/oxfw-hwdep.c)
> 

Yeah.  That's a good point.  I'll resend.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH v2 1/2] ALSA: oxfw: remove an unnecessary condition in hwdep_read()
  2021-01-22  7:13     ` Dan Carpenter
@ 2021-01-25 11:12       ` Dan Carpenter
  -1 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2021-01-25 11:12 UTC (permalink / raw)
  To: Clemens Ladisch, Christophe JAILLET
  Cc: alsa-devel, kernel-janitors, Takashi Iwai, Mark Brown

Smatch complains that "count" isn't clamped properly and
"oxfw->dev_lock_changed" is false then it leads to an information
leak.  But it turns out that "oxfw->dev_lock_changed" is always
set and the condition can be removed.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: this version just removes the condition

 sound/firewire/oxfw/oxfw-hwdep.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/sound/firewire/oxfw/oxfw-hwdep.c b/sound/firewire/oxfw/oxfw-hwdep.c
index 9e1b3e151bad..a0fe99618554 100644
--- a/sound/firewire/oxfw/oxfw-hwdep.c
+++ b/sound/firewire/oxfw/oxfw-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 (oxfw->dev_lock_changed) {
-		event.lock_status.type = SNDRV_FIREWIRE_EVENT_LOCK_STATUS;
-		event.lock_status.status = (oxfw->dev_lock_count > 0);
-		oxfw->dev_lock_changed = false;
+	event.lock_status.type = SNDRV_FIREWIRE_EVENT_LOCK_STATUS;
+	event.lock_status.status = (oxfw->dev_lock_count > 0);
+	oxfw->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(&oxfw->lock);
 
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH v2 1/2] ALSA: oxfw: remove an unnecessary condition in hwdep_read()
@ 2021-01-25 11:12       ` Dan Carpenter
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2021-01-25 11:12 UTC (permalink / raw)
  To: Clemens Ladisch, Christophe JAILLET
  Cc: alsa-devel, kernel-janitors, Takashi Iwai, Mark Brown

Smatch complains that "count" isn't clamped properly and
"oxfw->dev_lock_changed" is false then it leads to an information
leak.  But it turns out that "oxfw->dev_lock_changed" is always
set and the condition can be removed.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: this version just removes the condition

 sound/firewire/oxfw/oxfw-hwdep.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/sound/firewire/oxfw/oxfw-hwdep.c b/sound/firewire/oxfw/oxfw-hwdep.c
index 9e1b3e151bad..a0fe99618554 100644
--- a/sound/firewire/oxfw/oxfw-hwdep.c
+++ b/sound/firewire/oxfw/oxfw-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 (oxfw->dev_lock_changed) {
-		event.lock_status.type = SNDRV_FIREWIRE_EVENT_LOCK_STATUS;
-		event.lock_status.status = (oxfw->dev_lock_count > 0);
-		oxfw->dev_lock_changed = false;
+	event.lock_status.type = SNDRV_FIREWIRE_EVENT_LOCK_STATUS;
+	event.lock_status.status = (oxfw->dev_lock_count > 0);
+	oxfw->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(&oxfw->lock);
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH v2 2/2] ALSA: fireface: remove unnecessary condition in hwdep_read()
  2021-01-22  7:13     ` Dan Carpenter
@ 2021-01-25 11:13       ` Dan Carpenter
  -1 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2021-01-25 11:13 UTC (permalink / raw)
  To: Clemens Ladisch, Christophe JAILLET
  Cc: alsa-devel, kernel-janitors, Takashi Iwai, Mark Brown

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

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH v2 2/2] ALSA: fireface: remove unnecessary condition in hwdep_read()
@ 2021-01-25 11:13       ` Dan Carpenter
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2021-01-25 11:13 UTC (permalink / raw)
  To: Clemens Ladisch, Christophe JAILLET
  Cc: alsa-devel, kernel-janitors, Takashi Iwai, Mark Brown

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

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] ALSA: fireface: remove unnecessary condition in hwdep_read()
  2021-01-25 11:13       ` Dan Carpenter
@ 2021-01-25 13:45         ` Takashi Sakamoto
  -1 siblings, 0 replies; 18+ messages in thread
From: Takashi Sakamoto @ 2021-01-25 13:45 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: alsa-devel, kernel-janitors, Clemens Ladisch, Takashi Iwai,
	Mark Brown, Christophe JAILLET

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] ALSA: fireface: remove unnecessary condition in hwdep_read()
@ 2021-01-25 13:45         ` Takashi Sakamoto
  0 siblings, 0 replies; 18+ messages in thread
From: Takashi Sakamoto @ 2021-01-25 13:45 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: alsa-devel, kernel-janitors, Clemens Ladisch, Takashi Iwai,
	Mark Brown, Christophe JAILLET

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] ALSA: oxfw: remove an unnecessary condition in hwdep_read()
  2021-01-25 11:12       ` Dan Carpenter
@ 2021-01-25 13:46         ` Takashi Sakamoto
  -1 siblings, 0 replies; 18+ messages in thread
From: Takashi Sakamoto @ 2021-01-25 13:46 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: alsa-devel, kernel-janitors, Clemens Ladisch, Takashi Iwai,
	Mark Brown, Christophe JAILLET

On Mon, Jan 25, 2021 at 02:12:54PM +0300, Dan Carpenter wrote:
> Smatch complains that "count" isn't clamped properly and
> "oxfw->dev_lock_changed" is false then it leads to an information
> leak.  But it turns out that "oxfw->dev_lock_changed" is always
> set and the condition can be removed.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: this version just removes the condition
> 
>  sound/firewire/oxfw/oxfw-hwdep.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
 
Acked-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> 

> diff --git a/sound/firewire/oxfw/oxfw-hwdep.c b/sound/firewire/oxfw/oxfw-hwdep.c
> index 9e1b3e151bad..a0fe99618554 100644
> --- a/sound/firewire/oxfw/oxfw-hwdep.c
> +++ b/sound/firewire/oxfw/oxfw-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 (oxfw->dev_lock_changed) {
> -		event.lock_status.type = SNDRV_FIREWIRE_EVENT_LOCK_STATUS;
> -		event.lock_status.status = (oxfw->dev_lock_count > 0);
> -		oxfw->dev_lock_changed = false;
> +	event.lock_status.type = SNDRV_FIREWIRE_EVENT_LOCK_STATUS;
> +	event.lock_status.status = (oxfw->dev_lock_count > 0);
> +	oxfw->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(&oxfw->lock);
>  
> -- 
> 2.29.2

Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] ALSA: oxfw: remove an unnecessary condition in hwdep_read()
@ 2021-01-25 13:46         ` Takashi Sakamoto
  0 siblings, 0 replies; 18+ messages in thread
From: Takashi Sakamoto @ 2021-01-25 13:46 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: alsa-devel, kernel-janitors, Clemens Ladisch, Takashi Iwai,
	Mark Brown, Christophe JAILLET

On Mon, Jan 25, 2021 at 02:12:54PM +0300, Dan Carpenter wrote:
> Smatch complains that "count" isn't clamped properly and
> "oxfw->dev_lock_changed" is false then it leads to an information
> leak.  But it turns out that "oxfw->dev_lock_changed" is always
> set and the condition can be removed.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: this version just removes the condition
> 
>  sound/firewire/oxfw/oxfw-hwdep.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
 
Acked-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> 

> diff --git a/sound/firewire/oxfw/oxfw-hwdep.c b/sound/firewire/oxfw/oxfw-hwdep.c
> index 9e1b3e151bad..a0fe99618554 100644
> --- a/sound/firewire/oxfw/oxfw-hwdep.c
> +++ b/sound/firewire/oxfw/oxfw-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 (oxfw->dev_lock_changed) {
> -		event.lock_status.type = SNDRV_FIREWIRE_EVENT_LOCK_STATUS;
> -		event.lock_status.status = (oxfw->dev_lock_count > 0);
> -		oxfw->dev_lock_changed = false;
> +	event.lock_status.type = SNDRV_FIREWIRE_EVENT_LOCK_STATUS;
> +	event.lock_status.status = (oxfw->dev_lock_count > 0);
> +	oxfw->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(&oxfw->lock);
>  
> -- 
> 2.29.2

Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] ALSA: oxfw: remove an unnecessary condition in hwdep_read()
  2021-01-25 11:12       ` Dan Carpenter
@ 2021-01-25 13:51         ` Takashi Iwai
  -1 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2021-01-25 13:51 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: alsa-devel, kernel-janitors, Clemens Ladisch, Takashi Iwai,
	Mark Brown, Christophe JAILLET

On Mon, 25 Jan 2021 12:12:54 +0100,
Dan Carpenter wrote:
> 
> Smatch complains that "count" isn't clamped properly and
> "oxfw->dev_lock_changed" is false then it leads to an information
> leak.  But it turns out that "oxfw->dev_lock_changed" is always
> set and the condition can be removed.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: this version just removes the condition

Applied now.  Thanks.


Takashi

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] ALSA: oxfw: remove an unnecessary condition in hwdep_read()
@ 2021-01-25 13:51         ` Takashi Iwai
  0 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2021-01-25 13:51 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: alsa-devel, kernel-janitors, Clemens Ladisch, Takashi Iwai,
	Mark Brown, Christophe JAILLET

On Mon, 25 Jan 2021 12:12:54 +0100,
Dan Carpenter wrote:
> 
> Smatch complains that "count" isn't clamped properly and
> "oxfw->dev_lock_changed" is false then it leads to an information
> leak.  But it turns out that "oxfw->dev_lock_changed" is always
> set and the condition can be removed.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: this version just removes the condition

Applied now.  Thanks.


Takashi

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] ALSA: fireface: remove unnecessary condition in hwdep_read()
  2021-01-25 11:13       ` Dan Carpenter
@ 2021-01-25 13:51         ` Takashi Iwai
  -1 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2021-01-25 13:51 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: alsa-devel, kernel-janitors, Clemens Ladisch, Takashi Iwai,
	Mark Brown, Christophe JAILLET

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] ALSA: fireface: remove unnecessary condition in hwdep_read()
@ 2021-01-25 13:51         ` Takashi Iwai
  0 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2021-01-25 13:51 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: alsa-devel, kernel-janitors, Clemens Ladisch, Takashi Iwai,
	Mark Brown, Christophe JAILLET

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-01-25 13:53 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-21  6:10 [PATCH] ALSA: fireface: fix info leak in hwdep_read() Dan Carpenter
2021-01-21  6:10 ` Dan Carpenter
2021-01-21 20:42 ` Christophe JAILLET
2021-01-21 20:42   ` Christophe JAILLET
2021-01-22  7:13   ` Dan Carpenter
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 11:12       ` Dan Carpenter
2021-01-25 13:46       ` Takashi Sakamoto
2021-01-25 13:46         ` Takashi Sakamoto
2021-01-25 13:51       ` Takashi Iwai
2021-01-25 13:51         ` Takashi Iwai
2021-01-25 11:13     ` [PATCH v2 2/2] ALSA: fireface: remove " Dan Carpenter
2021-01-25 11:13       ` Dan Carpenter
2021-01-25 13:45       ` Takashi Sakamoto
2021-01-25 13:45         ` Takashi Sakamoto
2021-01-25 13:51       ` Takashi Iwai
2021-01-25 13:51         ` Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.