All of lore.kernel.org
 help / color / mirror / Atom feed
* sound/pci/ac97/ac97_patch.c: possible negative array index
@ 2006-11-07 18:03 Adrian Bunk
  2006-11-08 16:07   ` [Alsa-devel] " Takashi Iwai
  0 siblings, 1 reply; 4+ messages in thread
From: Adrian Bunk @ 2006-11-07 18:03 UTC (permalink / raw)
  To: perex, alsa-devel; +Cc: linux-kernel

The Coverity checker noted the following in sound/pci/ac97/ac97_patch.c:

<--  snip  -->

...
static int patch_ad1881_chained1(struct snd_ac97 * ac97, int idx, unsigned short codec_bits)
{
        static int cfg_bits[3] = { 1<<12, 1<<14, 1<<13 };
        unsigned short val;
        
        snd_ac97_update_bits(ac97, AC97_AD_SERIAL_CFG, 0x7000, cfg_bits[idx]);
        snd_ac97_write_cache(ac97, AC97_AD_CODEC_CFG, 0x0004);  // SDIE
        val = snd_ac97_read(ac97, AC97_VENDOR_ID2);
        if ((val & 0xff40) != 0x5340)
                return 0;
        if (codec_bits)
                snd_ac97_write_cache(ac97, AC97_AD_CODEC_CFG, codec_bits);
        ac97->spec.ad18xx.chained[idx] = cfg_bits[idx];
        ac97->spec.ad18xx.id[idx] = val;
        ac97->spec.ad18xx.codec_cfg[idx] = codec_bits ? codec_bits : 0x0004;
        return 1;
}

static void patch_ad1881_chained(struct snd_ac97 * ac97, int unchained_idx, int cidx1, int cidx2)
{
        // already detected?
        if (ac97->spec.ad18xx.unchained[cidx1] || ac97->spec.ad18xx.chained[cidx1])
                cidx1 = -1;
        if (ac97->spec.ad18xx.unchained[cidx2] || ac97->spec.ad18xx.chained[cidx2])
                cidx2 = -1;
        if (cidx1 < 0 && cidx2 < 0)
                return;
        // test for chained codecs
        snd_ac97_update_bits(ac97, AC97_AD_SERIAL_CFG, 0x7000,
                             ac97->spec.ad18xx.unchained[unchained_idx]);
        snd_ac97_write_cache(ac97, AC97_AD_CODEC_CFG, 0x0002);          // ID1C
        ac97->spec.ad18xx.codec_cfg[unchained_idx] = 0x0002;
        if (cidx1 >= 0) {
                if (patch_ad1881_chained1(ac97, cidx1, 0x0006))         // SDIE | ID1C
                        patch_ad1881_chained1(ac97, cidx2, 0);
                else if (patch_ad1881_chained1(ac97, cidx2, 0x0006))    // SDIE | ID1C
                        patch_ad1881_chained1(ac97, cidx1, 0);
        } else if (cidx2 >= 0) {
                patch_ad1881_chained1(ac97, cidx2, 0);
        }
}
...

<--  snip  -->

If there are in patch_ad1881_chained() (cidx2 == -1) and (cidx1 >= 0), 
-1 will be used as array index in patch_ad1881_chained1().

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 4+ messages in thread

* sound/pci/ac97/ac97_patch.c: possible negative array index
@ 2006-11-07 18:03 Adrian Bunk
  0 siblings, 0 replies; 4+ messages in thread
From: Adrian Bunk @ 2006-11-07 18:03 UTC (permalink / raw)
  To: perex, alsa-devel; +Cc: linux-kernel

The Coverity checker noted the following in sound/pci/ac97/ac97_patch.c:

<--  snip  -->

...
static int patch_ad1881_chained1(struct snd_ac97 * ac97, int idx, unsigned short codec_bits)
{
        static int cfg_bits[3] = { 1<<12, 1<<14, 1<<13 };
        unsigned short val;
        
        snd_ac97_update_bits(ac97, AC97_AD_SERIAL_CFG, 0x7000, cfg_bits[idx]);
        snd_ac97_write_cache(ac97, AC97_AD_CODEC_CFG, 0x0004);  // SDIE
        val = snd_ac97_read(ac97, AC97_VENDOR_ID2);
        if ((val & 0xff40) != 0x5340)
                return 0;
        if (codec_bits)
                snd_ac97_write_cache(ac97, AC97_AD_CODEC_CFG, codec_bits);
        ac97->spec.ad18xx.chained[idx] = cfg_bits[idx];
        ac97->spec.ad18xx.id[idx] = val;
        ac97->spec.ad18xx.codec_cfg[idx] = codec_bits ? codec_bits : 0x0004;
        return 1;
}

static void patch_ad1881_chained(struct snd_ac97 * ac97, int unchained_idx, int cidx1, int cidx2)
{
        // already detected?
        if (ac97->spec.ad18xx.unchained[cidx1] || ac97->spec.ad18xx.chained[cidx1])
                cidx1 = -1;
        if (ac97->spec.ad18xx.unchained[cidx2] || ac97->spec.ad18xx.chained[cidx2])
                cidx2 = -1;
        if (cidx1 < 0 && cidx2 < 0)
                return;
        // test for chained codecs
        snd_ac97_update_bits(ac97, AC97_AD_SERIAL_CFG, 0x7000,
                             ac97->spec.ad18xx.unchained[unchained_idx]);
        snd_ac97_write_cache(ac97, AC97_AD_CODEC_CFG, 0x0002);          // ID1C
        ac97->spec.ad18xx.codec_cfg[unchained_idx] = 0x0002;
        if (cidx1 >= 0) {
                if (patch_ad1881_chained1(ac97, cidx1, 0x0006))         // SDIE | ID1C
                        patch_ad1881_chained1(ac97, cidx2, 0);
                else if (patch_ad1881_chained1(ac97, cidx2, 0x0006))    // SDIE | ID1C
                        patch_ad1881_chained1(ac97, cidx1, 0);
        } else if (cidx2 >= 0) {
                patch_ad1881_chained1(ac97, cidx2, 0);
        }
}
...

<--  snip  -->

If there are in patch_ad1881_chained() (cidx2 == -1) and (cidx1 >= 0), 
-1 will be used as array index in patch_ad1881_chained1().

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: sound/pci/ac97/ac97_patch.c: possible negative array index
  2006-11-07 18:03 sound/pci/ac97/ac97_patch.c: possible negative array index Adrian Bunk
@ 2006-11-08 16:07   ` Takashi Iwai
  0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2006-11-08 16:07 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: alsa-devel, linux-kernel, perex

At Tue, 7 Nov 2006 19:03:23 +0100,
Adrian Bunk wrote:
> 
> The Coverity checker noted the following in sound/pci/ac97/ac97_patch.c:
> 
> <--  snip  -->
> 
> ...
> static int patch_ad1881_chained1(struct snd_ac97 * ac97, int idx, unsigned short codec_bits)
> {
>         static int cfg_bits[3] = { 1<<12, 1<<14, 1<<13 };
>         unsigned short val;
>         
>         snd_ac97_update_bits(ac97, AC97_AD_SERIAL_CFG, 0x7000, cfg_bits[idx]);
>         snd_ac97_write_cache(ac97, AC97_AD_CODEC_CFG, 0x0004);  // SDIE
>         val = snd_ac97_read(ac97, AC97_VENDOR_ID2);
>         if ((val & 0xff40) != 0x5340)
>                 return 0;
>         if (codec_bits)
>                 snd_ac97_write_cache(ac97, AC97_AD_CODEC_CFG, codec_bits);
>         ac97->spec.ad18xx.chained[idx] = cfg_bits[idx];
>         ac97->spec.ad18xx.id[idx] = val;
>         ac97->spec.ad18xx.codec_cfg[idx] = codec_bits ? codec_bits : 0x0004;
>         return 1;
> }
> 
> static void patch_ad1881_chained(struct snd_ac97 * ac97, int unchained_idx, int cidx1, int cidx2)
> {
>         // already detected?
>         if (ac97->spec.ad18xx.unchained[cidx1] || ac97->spec.ad18xx.chained[cidx1])
>                 cidx1 = -1;
>         if (ac97->spec.ad18xx.unchained[cidx2] || ac97->spec.ad18xx.chained[cidx2])
>                 cidx2 = -1;
>         if (cidx1 < 0 && cidx2 < 0)
>                 return;
>         // test for chained codecs
>         snd_ac97_update_bits(ac97, AC97_AD_SERIAL_CFG, 0x7000,
>                              ac97->spec.ad18xx.unchained[unchained_idx]);
>         snd_ac97_write_cache(ac97, AC97_AD_CODEC_CFG, 0x0002);          // ID1C
>         ac97->spec.ad18xx.codec_cfg[unchained_idx] = 0x0002;
>         if (cidx1 >= 0) {
>                 if (patch_ad1881_chained1(ac97, cidx1, 0x0006))         // SDIE | ID1C
>                         patch_ad1881_chained1(ac97, cidx2, 0);
>                 else if (patch_ad1881_chained1(ac97, cidx2, 0x0006))    // SDIE | ID1C
>                         patch_ad1881_chained1(ac97, cidx1, 0);
>         } else if (cidx2 >= 0) {
>                 patch_ad1881_chained1(ac97, cidx2, 0);
>         }
> }
> ...
> 
> <--  snip  -->
> 
> If there are in patch_ad1881_chained() (cidx2 == -1) and (cidx1 >= 0), 
> -1 will be used as array index in patch_ad1881_chained1().

Thanks, fixed on ALSA tree now.


Takashi

diff -r d152dacb9bad -r 4fc4f9aba378 sound/pci/ac97/ac97_patch.c
--- a/sound/pci/ac97/ac97_patch.c	Wed Nov 08 15:41:29 2006 +0100
+++ b/sound/pci/ac97/ac97_patch.c	Wed Nov 08 15:48:43 2006 +0100
@@ -1467,7 +1467,9 @@ static void patch_ad1881_chained(struct 
 	snd_ac97_write_cache(ac97, AC97_AD_CODEC_CFG, 0x0002);		// ID1C
 	ac97->spec.ad18xx.codec_cfg[unchained_idx] = 0x0002;
 	if (cidx1 >= 0) {
-		if (patch_ad1881_chained1(ac97, cidx1, 0x0006))		// SDIE | ID1C
+		if (cidx2 < 0)
+			patch_ad1881_chained1(ac97, cidx1, 0);
+		else if (patch_ad1881_chained1(ac97, cidx1, 0x0006))	// SDIE | ID1C
 			patch_ad1881_chained1(ac97, cidx2, 0);
 		else if (patch_ad1881_chained1(ac97, cidx2, 0x0006))	// SDIE | ID1C
 			patch_ad1881_chained1(ac97, cidx1, 0);

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Alsa-devel] sound/pci/ac97/ac97_patch.c: possible negative array index
@ 2006-11-08 16:07   ` Takashi Iwai
  0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2006-11-08 16:07 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: perex, alsa-devel, linux-kernel

At Tue, 7 Nov 2006 19:03:23 +0100,
Adrian Bunk wrote:
> 
> The Coverity checker noted the following in sound/pci/ac97/ac97_patch.c:
> 
> <--  snip  -->
> 
> ...
> static int patch_ad1881_chained1(struct snd_ac97 * ac97, int idx, unsigned short codec_bits)
> {
>         static int cfg_bits[3] = { 1<<12, 1<<14, 1<<13 };
>         unsigned short val;
>         
>         snd_ac97_update_bits(ac97, AC97_AD_SERIAL_CFG, 0x7000, cfg_bits[idx]);
>         snd_ac97_write_cache(ac97, AC97_AD_CODEC_CFG, 0x0004);  // SDIE
>         val = snd_ac97_read(ac97, AC97_VENDOR_ID2);
>         if ((val & 0xff40) != 0x5340)
>                 return 0;
>         if (codec_bits)
>                 snd_ac97_write_cache(ac97, AC97_AD_CODEC_CFG, codec_bits);
>         ac97->spec.ad18xx.chained[idx] = cfg_bits[idx];
>         ac97->spec.ad18xx.id[idx] = val;
>         ac97->spec.ad18xx.codec_cfg[idx] = codec_bits ? codec_bits : 0x0004;
>         return 1;
> }
> 
> static void patch_ad1881_chained(struct snd_ac97 * ac97, int unchained_idx, int cidx1, int cidx2)
> {
>         // already detected?
>         if (ac97->spec.ad18xx.unchained[cidx1] || ac97->spec.ad18xx.chained[cidx1])
>                 cidx1 = -1;
>         if (ac97->spec.ad18xx.unchained[cidx2] || ac97->spec.ad18xx.chained[cidx2])
>                 cidx2 = -1;
>         if (cidx1 < 0 && cidx2 < 0)
>                 return;
>         // test for chained codecs
>         snd_ac97_update_bits(ac97, AC97_AD_SERIAL_CFG, 0x7000,
>                              ac97->spec.ad18xx.unchained[unchained_idx]);
>         snd_ac97_write_cache(ac97, AC97_AD_CODEC_CFG, 0x0002);          // ID1C
>         ac97->spec.ad18xx.codec_cfg[unchained_idx] = 0x0002;
>         if (cidx1 >= 0) {
>                 if (patch_ad1881_chained1(ac97, cidx1, 0x0006))         // SDIE | ID1C
>                         patch_ad1881_chained1(ac97, cidx2, 0);
>                 else if (patch_ad1881_chained1(ac97, cidx2, 0x0006))    // SDIE | ID1C
>                         patch_ad1881_chained1(ac97, cidx1, 0);
>         } else if (cidx2 >= 0) {
>                 patch_ad1881_chained1(ac97, cidx2, 0);
>         }
> }
> ...
> 
> <--  snip  -->
> 
> If there are in patch_ad1881_chained() (cidx2 == -1) and (cidx1 >= 0), 
> -1 will be used as array index in patch_ad1881_chained1().

Thanks, fixed on ALSA tree now.


Takashi

diff -r d152dacb9bad -r 4fc4f9aba378 sound/pci/ac97/ac97_patch.c
--- a/sound/pci/ac97/ac97_patch.c	Wed Nov 08 15:41:29 2006 +0100
+++ b/sound/pci/ac97/ac97_patch.c	Wed Nov 08 15:48:43 2006 +0100
@@ -1467,7 +1467,9 @@ static void patch_ad1881_chained(struct 
 	snd_ac97_write_cache(ac97, AC97_AD_CODEC_CFG, 0x0002);		// ID1C
 	ac97->spec.ad18xx.codec_cfg[unchained_idx] = 0x0002;
 	if (cidx1 >= 0) {
-		if (patch_ad1881_chained1(ac97, cidx1, 0x0006))		// SDIE | ID1C
+		if (cidx2 < 0)
+			patch_ad1881_chained1(ac97, cidx1, 0);
+		else if (patch_ad1881_chained1(ac97, cidx1, 0x0006))	// SDIE | ID1C
 			patch_ad1881_chained1(ac97, cidx2, 0);
 		else if (patch_ad1881_chained1(ac97, cidx2, 0x0006))	// SDIE | ID1C
 			patch_ad1881_chained1(ac97, cidx1, 0);

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2006-11-08 16:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-11-07 18:03 sound/pci/ac97/ac97_patch.c: possible negative array index Adrian Bunk
2006-11-08 16:07 ` Takashi Iwai
2006-11-08 16:07   ` [Alsa-devel] " Takashi Iwai
  -- strict thread matches above, loose matches on Subject: below --
2006-11-07 18:03 Adrian Bunk

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.