Takashi Iwai schrieb:
At Wed, 8 Sep 2010 09:26:32 +0200, Dan Carpenter wrote:
We don't want "pre_event_timeout" to be negative because that would result in a stack traces in dmesg when we schedule a negative timeout. In the original code "HZ * val" could overflow so I just moved the check for negative below the multiply.
This would bring another side-effect. When a value like 0x80001234 is passed, this would result in a positive value in turn. We need additional check like below.
thanks,
Takashi
diff --git a/sound/core/seq/oss/seq_oss_ioctl.c b/sound/core/seq/oss/seq_oss_ioctl.c index 5ac701c..b2e0789 100644 --- a/sound/core/seq/oss/seq_oss_ioctl.c +++ b/sound/core/seq/oss/seq_oss_ioctl.c @@ -191,10 +191,13 @@ snd_seq_oss_ioctl(struct seq_oss_devinfo *dp, unsigned int cmd, unsigned long ca return 0; if (get_user(val, p)) return -EFAULT;
if (val <= 0)val = -1;else
if (val < 0)val = 0;else { val = (HZ * val) / 10;if (val < 0) /* check overflow */val = 0;}diff --git a/sound/oss/midibuf.c b/sound/oss/midibuf.c index 782b3b8..b8da210 100644 --- a/sound/oss/midibuf.c +++ b/sound/oss/midibuf.c @@ -382,7 +382,11 @@ int MIDIbuf_ioctl(int dev, struct file *file, return -EFAULT; if (val < 0) val = 0;
val = (HZ * val) / 10;
else {val = (HZ * val) / 10;if (val < 0) /* check overflow */val = 0;} parms[dev].prech_timeout = val; return put_user(val, (int __user *)arg);diff --git a/sound/oss/sequencer.c b/sound/oss/sequencer.c index e85789e..f579210 100644 --- a/sound/oss/sequencer.c +++ b/sound/oss/sequencer.c @@ -1509,7 +1509,11 @@ int sequencer_ioctl(int dev, struct file *file, unsigned int cmd, void __user *a return -EFAULT; if (val < 0) val = 0;
val = (HZ * val) / 10;
else {val = (HZ * val) / 10;if (val < 0) /* check overflow */val = 0;} pre_event_timeout = val; break;--
Perhaps a recalc_val() is here better ? That would avoid duplication and make sure that they behave equal. (see: > - if (val <= 0) > - val = -1; )
int recalc_val(int hz, int val) { if (val < 0) return 0 val = (hz * val) / 10; if (val < 0) return 0
return val; }
just my 2 cents, not a tested patch. re, wh