* [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
* 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
* [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 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.