* [patch] [ALSA] hwdep: silence integer overflow warning
@ 2011-10-20 6:09 Dan Carpenter
[not found] ` <4E9FD02B.6030307@bfs.de>
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2011-10-20 6:09 UTC (permalink / raw)
To: Jaroslav Kysela; +Cc: Takashi Iwai, Paul Gortmaker, alsa-devel, kernel-janitors
Smatch complains that if device is INT_MAX then device + 1 can
overflow. It just means we would have an annoying loop while we
check all the devices from -2147483648 to SNDRV_MINOR_HWDEPS.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/sound/core/hwdep.c b/sound/core/hwdep.c
index c7ceb28..edfdf6d 100644
--- a/sound/core/hwdep.c
+++ b/sound/core/hwdep.c
@@ -273,7 +273,14 @@ static int snd_hwdep_control_ioctl(struct snd_card *card,
if (get_user(device, (int __user *)arg))
return -EFAULT;
mutex_lock(®ister_mutex);
- device = device < 0 ? 0 : device + 1;
+
+ if (device < 0)
+ device = 0;
+ else if (device > SNDRV_MINOR_HWDEPS)
+ device = SNDRV_MINOR_HWDEPS;
+ else
+ device++;
+
while (device < SNDRV_MINOR_HWDEPS) {
if (snd_hwdep_search(card, device))
break;
^ permalink raw reply related [flat|nested] 6+ messages in thread[parent not found: <4E9FD02B.6030307@bfs.de>]
* Re: [patch] [ALSA] hwdep: silence integer overflow warning [not found] ` <4E9FD02B.6030307@bfs.de> @ 2011-10-28 6:45 ` Dan Carpenter 2011-10-28 7:33 ` walter harms 2011-10-28 6:46 ` [patch v2] " Dan Carpenter 1 sibling, 1 reply; 6+ messages in thread From: Dan Carpenter @ 2011-10-28 6:45 UTC (permalink / raw) To: walter harms; +Cc: Takashi Iwai, Paul Gortmaker, alsa-devel, kernel-janitors On Thu, Oct 20, 2011 at 09:39:23AM +0200, walter harms wrote: > I am not sure how SNDRV_MINOR_HWDEPS is used further Either device = SNDRV_MINOR_HWDEPS or device = SNDRV_MINOR_HWDEPS + 1 are fine (equivalent) but it's a fair point. Your version is nicer. I'll redo the patch. regards, dan carpenter ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] [ALSA] hwdep: silence integer overflow warning 2011-10-28 6:45 ` Dan Carpenter @ 2011-10-28 7:33 ` walter harms 2011-10-28 8:55 ` Dan Carpenter 0 siblings, 1 reply; 6+ messages in thread From: walter harms @ 2011-10-28 7:33 UTC (permalink / raw) To: Dan Carpenter; +Cc: Takashi Iwai, Paul Gortmaker, alsa-devel, kernel-janitors Am 28.10.2011 08:45, schrieb Dan Carpenter: > On Thu, Oct 20, 2011 at 09:39:23AM +0200, walter harms wrote: >> I am not sure how SNDRV_MINOR_HWDEPS is used further > > Either device = SNDRV_MINOR_HWDEPS or device = SNDRV_MINOR_HWDEPS + 1 > are fine (equivalent) but it's a fair point. Your version is nicer. > > I'll redo the patch. > An other question is: Should the user be informed that the device is modified ? The Problem i see is that a user parameter is silently modified and buggy code will come through. re, wh ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] [ALSA] hwdep: silence integer overflow warning 2011-10-28 7:33 ` walter harms @ 2011-10-28 8:55 ` Dan Carpenter 0 siblings, 0 replies; 6+ messages in thread From: Dan Carpenter @ 2011-10-28 8:55 UTC (permalink / raw) To: walter harms; +Cc: Takashi Iwai, Paul Gortmaker, alsa-devel, kernel-janitors On Fri, Oct 28, 2011 at 09:33:11AM +0200, walter harms wrote: > > > Am 28.10.2011 08:45, schrieb Dan Carpenter: > > On Thu, Oct 20, 2011 at 09:39:23AM +0200, walter harms wrote: > >> I am not sure how SNDRV_MINOR_HWDEPS is used further > > > > Either device = SNDRV_MINOR_HWDEPS or device = SNDRV_MINOR_HWDEPS + 1 > > are fine (equivalent) but it's a fair point. Your version is nicer. > > > > I'll redo the patch. > > > > An other question is: > Should the user be informed that the device is modified ? > The Problem i see is that a user parameter is silently modified > and buggy code will come through. That would be an API change. This is cut and pasted code, and we've had this exact discussion before for sound/core/rawmidi.c a year ago. You were involved in the discussion too. :P http://marc.info/?l=kernel-janitors&m=128401699203676&w=2 regards, dan carpenter ^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch v2] [ALSA] hwdep: silence integer overflow warning [not found] ` <4E9FD02B.6030307@bfs.de> 2011-10-28 6:45 ` Dan Carpenter @ 2011-10-28 6:46 ` Dan Carpenter 2011-10-31 8:55 ` Takashi Iwai 1 sibling, 1 reply; 6+ messages in thread From: Dan Carpenter @ 2011-10-28 6:46 UTC (permalink / raw) To: walter harms; +Cc: Takashi Iwai, Paul Gortmaker, alsa-devel, kernel-janitors Smatch complains that if device is INT_MAX then device + 1 can overflow. It just means we would have an annoying loop while we check all the devices from -2147483648 to SNDRV_MINOR_HWDEPS. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- v2: reshuffled it a bit per Walter Harms's suggestion. diff --git a/sound/core/hwdep.c b/sound/core/hwdep.c index c7ceb28..75ea16f 100644 --- a/sound/core/hwdep.c +++ b/sound/core/hwdep.c @@ -273,7 +273,14 @@ static int snd_hwdep_control_ioctl(struct snd_card *card, if (get_user(device, (int __user *)arg)) return -EFAULT; mutex_lock(®ister_mutex); - device = device < 0 ? 0 : device + 1; + + if (device < 0) + device = 0; + else if (device < SNDRV_MINOR_HWDEPS) + device++; + else + device = SNDRV_MINOR_HWDEPS; + while (device < SNDRV_MINOR_HWDEPS) { if (snd_hwdep_search(card, device)) break; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [patch v2] [ALSA] hwdep: silence integer overflow warning 2011-10-28 6:46 ` [patch v2] " Dan Carpenter @ 2011-10-31 8:55 ` Takashi Iwai 0 siblings, 0 replies; 6+ messages in thread From: Takashi Iwai @ 2011-10-31 8:55 UTC (permalink / raw) To: Dan Carpenter; +Cc: Paul Gortmaker, alsa-devel, kernel-janitors, walter harms At Fri, 28 Oct 2011 09:46:01 +0300, Dan Carpenter wrote: > > Smatch complains that if device is INT_MAX then device + 1 can > overflow. It just means we would have an annoying loop while we > check all the devices from -2147483648 to SNDRV_MINOR_HWDEPS. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Applied now. Thanks! Takashi > --- > v2: reshuffled it a bit per Walter Harms's suggestion. > > diff --git a/sound/core/hwdep.c b/sound/core/hwdep.c > index c7ceb28..75ea16f 100644 > --- a/sound/core/hwdep.c > +++ b/sound/core/hwdep.c > @@ -273,7 +273,14 @@ static int snd_hwdep_control_ioctl(struct snd_card *card, > if (get_user(device, (int __user *)arg)) > return -EFAULT; > mutex_lock(®ister_mutex); > - device = device < 0 ? 0 : device + 1; > + > + if (device < 0) > + device = 0; > + else if (device < SNDRV_MINOR_HWDEPS) > + device++; > + else > + device = SNDRV_MINOR_HWDEPS; > + > while (device < SNDRV_MINOR_HWDEPS) { > if (snd_hwdep_search(card, device)) > break; > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-10-31 8:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-20 6:09 [patch] [ALSA] hwdep: silence integer overflow warning Dan Carpenter
[not found] ` <4E9FD02B.6030307@bfs.de>
2011-10-28 6:45 ` Dan Carpenter
2011-10-28 7:33 ` walter harms
2011-10-28 8:55 ` Dan Carpenter
2011-10-28 6:46 ` [patch v2] " Dan Carpenter
2011-10-31 8:55 ` Takashi Iwai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox