[PATCH] hda empty modelname comparison

[PATCH] hda empty modelname comparison

[Joachim Deguara]

[-- Attachment #1: Type: text/plain, Size: 102 bytes --]

This patch prevents a comparison between a possibly empty modelname
string in the hda code.

-Joachim

[-- Attachment #2: hda_empty_modelname_comparison.patch --]
[-- Type: text/x-patch, Size: 656 bytes --]

Only in alsa-kernel/include: version.h
diff -rup /mnt/alsa/alsa-kernel/pci/hda/hda_codec.c alsa-kernel/pci/hda/hda_codec.c
--- /mnt/alsa/alsa-kernel/pci/hda/hda_codec.c	2007-03-01 14:50:24.000000000 +0100
+++ alsa-kernel/pci/hda/hda_codec.c	2007-03-06 15:37:03.000000000 +0100
@@ -573,7 +573,7 @@ int snd_hda_codec_new(struct hda_bus *bu
 							 0);
 	}
 
-	if (strcmp(codec->bus->modelname, "generic"))
+	if ( codec->bus->modelname && strcmp(codec->bus->modelname, "generic"))
 		codec->preset = find_codec_preset(codec);
 	if (! *bus->card->mixername)
 		snd_hda_get_codec_name(codec, bus->card->mixername,
Only in alsa-kernel/pci/hda: .hda_codec.c.swp

[-- Attachment #3: Type: text/plain, Size: 345 bytes --]

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[-- Attachment #4: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/alsa-devel

Re: [PATCH] hda empty modelname comparison

[Tobin Davis]

[-- Attachment #1.1: Type: text/plain, Size: 630 bytes --]

On Tue, 2007-03-06 at 15:59 +0100, Joachim Deguara wrote:

> This patch prevents a comparison between a possibly empty modelname
> string in the hda code.
> 
> -Joachim


Unfortunately, the way the original patch was written, the only options
are generic or segfault.  Your patch just removes the segfault, sending
all cards to the generic parser.

I've tested a slightly different approach, based on your empty test,
just reverse logic.  Test it to confirm that it works.  It works on my
system with a Conexant reference board and a Sigmatel STAC9227 (both on
the same system).



Signed off by Tobin Davis <tdavis@dsl-only.net>

[-- Attachment #1.2: Type: text/html, Size: 1119 bytes --]

[-- Attachment #2: generic-fix.patch --]
[-- Type: text/x-patch, Size: 490 bytes --]

diff -r bf193701a812 pci/hda/hda_codec.c
--- a/pci/hda/hda_codec.c	Mon Feb 26 16:07:42 2007 +0100
+++ b/pci/hda/hda_codec.c	Wed Mar 07 12:02:37 2007 -0800
@@ -573,7 +573,7 @@ int snd_hda_codec_new(struct hda_bus *bu
 							 0);
 	}
 
-	if (strcmp(codec->bus->modelname, "generic"))
+	if (!(codec->bus->modelname && strcmp(codec->bus->modelname, "generic")))
 		codec->preset = find_codec_preset(codec);
 	if (! *bus->card->mixername)
 		snd_hda_get_codec_name(codec, bus->card->mixername,

[-- Attachment #3: Type: text/plain, Size: 345 bytes --]

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[-- Attachment #4: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/alsa-devel

Re: [PATCH] hda empty modelname comparison

[Takashi Iwai]

At Wed, 07 Mar 2007 12:04:30 -0800,
Tobin Davis wrote:
> 
> On Tue, 2007-03-06 at 15:59 +0100, Joachim Deguara wrote:
> 
>     This patch prevents a comparison between a possibly empty modelname
>     string in the hda code.
>     
>     -Joachim
> 
> Unfortunately, the way the original patch was written, the only options are
> generic or segfault.  Your patch just removes the segfault, sending all cards
> to the generic parser.
> 
> I've tested a slightly different approach, based on your empty test, just
> reverse logic.  Test it to confirm that it works.  It works on my system with
> a Conexant reference board and a Sigmatel STAC9227 (both on the same system).
> 
> Signed off by Tobin Davis <tdavis@dsl-only.net>                               
That's my bad.  I haven't tested much, and unfortunately, 1.0.14rc3
was released while I've been in vacation soon after the wrong patch.

The fix patch below is already on my local tree, but it couldn't be
pushed because the ALSA HG tree is currently down (I guess Jaroslav is
working on his server).  If needed, we'll release the fixed version.
So stay tuned.


Takashi

# HG changeset patch
# User tiwai
# Date 1173279359 -3600
# Node ID 9b8fef0370f213c42fc93098c7b8cc2c8188b177
# Parent  e4f79cb873e55818b4aca604d17e350b5a480a27
Fix NULL dereference with null modelname

Fix the NULL dereference of modelname option.  The check is moved to
find_codec_preset() now, too.

diff -r e4f79cb873e5 -r 9b8fef0370f2 pci/hda/hda_codec.c
--- a/pci/hda/hda_codec.c	Tue Mar 06 11:29:05 2007 +0100
+++ b/pci/hda/hda_codec.c	Wed Mar 07 15:55:59 2007 +0100
@@ -403,6 +403,9 @@ static const struct hda_codec_preset *fi
 {
 	const struct hda_codec_preset **tbl, *preset;
 
+	if (codec->bus->modelname && !strcmp(codec->bus->modelname, "generic"))
+		return NULL; /* use the generic parser */
+
 	for (tbl = hda_preset_tables; *tbl; tbl++) {
 		for (preset = *tbl; preset->id; preset++) {
 			u32 mask = preset->mask;
@@ -573,8 +576,7 @@ int snd_hda_codec_new(struct hda_bus *bu
 							 0);
 	}
 
-	if (strcmp(codec->bus->modelname, "generic"))
-		codec->preset = find_codec_preset(codec);
+	codec->preset = find_codec_preset(codec);
 	if (! *bus->card->mixername)
 		snd_hda_get_codec_name(codec, bus->card->mixername,
 				       sizeof(bus->card->mixername));

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: [PATCH] hda empty modelname comparison

[Tobin Davis]

[-- Attachment #1.1: Type: text/plain, Size: 426 bytes --]

On Wed, 2007-03-07 at 22:20 +0100, Takashi Iwai wrote:

> +       if (codec->bus->modelname && !strcmp(codec->bus->modelname,
> "generic"))
> +               return NULL; /* use the generic parser */
> 


Should be:

+       if (codec->bus->modelname && strcmp(codec->bus->modelname,
"generic"))
+               return NULL; /* use the generic parser */


You've got the logic reversed.

-- 
Tobin Davis <tdavis@dsl-only.net>

[-- Attachment #1.2: Type: text/html, Size: 1250 bytes --]

[-- Attachment #2: Type: text/plain, Size: 345 bytes --]

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[-- Attachment #3: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/alsa-devel

Re: [PATCH] hda empty modelname comparison

[Tobin Davis]

[-- Attachment #1.1: Type: text/plain, Size: 561 bytes --]

Oops, nevermind.  I've been reading too many pseudo-code documents.  I
expected strcmp to return true if matched.

This works.

Tobin

On Wed, 2007-03-07 at 22:20 +0100, Takashi Iwai wrote:

> +       if (codec->bus->modelname && !strcmp(codec->bus->modelname,
> "generic"))
> +               return NULL; /* use the generic parser */
> 


Should be:

+       if (codec->bus->modelname && strcmp(codec->bus->modelname,
"generic"))
+               return NULL; /* use the generic parser */


You've got the logic reversed.

-- 
Tobin Davis <tdavis@dsl-only.net>

[-- Attachment #1.2: Type: text/html, Size: 1419 bytes --]

[-- Attachment #2: Type: text/plain, Size: 345 bytes --]

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[-- Attachment #3: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/alsa-devel

Re: [PATCH] hda empty modelname comparison

[Takashi Iwai]

At Wed, 07 Mar 2007 22:20:20 +0100,
I wrote:
> 
> At Wed, 07 Mar 2007 12:04:30 -0800,
> Tobin Davis wrote:
> > 
> > On Tue, 2007-03-06 at 15:59 +0100, Joachim Deguara wrote:
> > 
> >     This patch prevents a comparison between a possibly empty modelname
> >     string in the hda code.
> >     
> >     -Joachim
> > 
> > Unfortunately, the way the original patch was written, the only options are
> > generic or segfault.  Your patch just removes the segfault, sending all cards
> > to the generic parser.
> > 
> > I've tested a slightly different approach, based on your empty test, just
> > reverse logic.  Test it to confirm that it works.  It works on my system with
> > a Conexant reference board and a Sigmatel STAC9227 (both on the same system).
> > 
> > Signed off by Tobin Davis <tdavis@dsl-only.net>                               
> That's my bad.  I haven't tested much, and unfortunately, 1.0.14rc3
> was released while I've been in vacation soon after the wrong patch.
> 
> The fix patch below is already on my local tree, but it couldn't be
> pushed because the ALSA HG tree is currently down (I guess Jaroslav is
> working on his server).  If needed, we'll release the fixed version.
> So stay tuned.

Now fixed on HG tree.


Takashi

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV