| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <s5h7iqqhb09.wl%tiwai@suse.de>
Date: Wed, 30 May 2007 12:16:38 +0200
From: Takashi Iwai <tiwai@...e.de>
To: Bill Nottingham <notting@...hat.com>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH] alsa: fix comparison of unsigned < 0
Hi,
thanks for checking this.
They seem actually hitting some real bugs.
At Wed, 30 May 2007 03:35:54 -0400,
Bill Nottingham wrote:
>
> diff -ru linux-2.6.21-old/sound/pci/ac97/ac97_patch.c linux-2.6.21/sound/pci/ac97/ac97_patch.c
> --- linux-2.6.21-old/sound/pci/ac97/ac97_patch.c 2007-05-30 02:53:05.000000000 -0400
> +++ linux-2.6.21/sound/pci/ac97/ac97_patch.c 2007-05-30 02:32:41.000000000 -0400
> @@ -2086,8 +2086,7 @@
> struct snd_ac97 *ac97 = snd_kcontrol_chip(kcontrol);
> unsigned short val;
>
> - if (ucontrol->value.enumerated.item[0] > 3
> - || ucontrol->value.enumerated.item[0] < 0)
> + if (ucontrol->value.enumerated.item[0] > 3)
> return -EINVAL;
> val = ctrl2reg[ucontrol->value.enumerated.item[0]]
> << AC97_AD198X_VREF_SHIFT;
This one is fine.
> diff -ru linux-2.6.21-old/sound/pci/ali5451/ali5451.c linux-2.6.21/sound/pci/ali5451/ali5451.c
> --- linux-2.6.21-old/sound/pci/ali5451/ali5451.c 2007-05-30 02:53:05.000000000 -0400
> +++ linux-2.6.21/sound/pci/ali5451/ali5451.c 2007-05-30 02:33:27.000000000 -0400
> @@ -2057,10 +2057,8 @@
> {
> if (codec->hw_initialized)
> snd_ali_disable_address_interrupt(codec);
> - if (codec->irq >= 0) {
> - synchronize_irq(codec->irq);
> - free_irq(codec->irq, codec);
> - }
> + synchronize_irq(codec->irq);
> + free_irq(codec->irq, codec);
> if (codec->port)
> pci_release_regions(codec->pci);
> pci_disable_device(codec->pci);
Bah, the check isn't wrong but the type of codec->irq.
It should be int instead of unsigned long. I'll fix it.
> diff -ru linux-2.6.21-old/sound/pci/ca0106/ca0106_proc.c linux-2.6.21/sound/pci/ca0106/ca0106_proc.c
> --- linux-2.6.21-old/sound/pci/ca0106/ca0106_proc.c 2007-05-30 02:53:05.000000000 -0400
> +++ linux-2.6.21/sound/pci/ca0106/ca0106_proc.c 2007-05-30 02:34:19.000000000 -0400
> @@ -305,7 +305,7 @@
> while (!snd_info_get_line(buffer, line, sizeof(line))) {
> if (sscanf(line, "%x %x", ®, &val) != 2)
> continue;
> - if ((reg < 0x40) && (reg >=0) && (val <= 0xffffffff) ) {
> + if ((reg < 0x40) && (val <= 0xffffffff) ) {
It's a 32bit unsigned int, so the check of 0xffffffff isn't needed,
too.
> spin_lock_irqsave(&emu->emu_lock, flags);
> outl(val, emu->port + (reg & 0xfffffffc));
> spin_unlock_irqrestore(&emu->emu_lock, flags);
> @@ -406,7 +406,7 @@
> while (!snd_info_get_line(buffer, line, sizeof(line))) {
> if (sscanf(line, "%x %x %x", ®, &channel_id, &val) != 3)
> continue;
> - if ((reg < 0x80) && (reg >=0) && (val <= 0xffffffff) && (channel_id >=0) && (channel_id <= 3) )
> + if ((reg < 0x80) && (val <= 0xffffffff) && (channel_id <= 3) )
> snd_ca0106_ptr_write(emu, reg, channel_id, val);
> }
> }
Ditto.
> diff -ru linux-2.6.21-old/sound/pci/rme9652/rme9652.c linux-2.6.21/sound/pci/rme9652/rme9652.c
> --- linux-2.6.21-old/sound/pci/rme9652/rme9652.c 2007-05-30 02:53:05.000000000 -0400
> +++ linux-2.6.21/sound/pci/rme9652/rme9652.c 2007-05-30 02:35:16.000000000 -0400
> @@ -406,8 +406,6 @@
> } else if (!frag)
> return 0;
> offset -= rme9652->max_jitter;
> - if (offset < 0)
> - offset += period_size * 2;
I think this check is actually needed, but it must be cast to int.
Will fix this, too.
Takashi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists