2006-03-06 09:05:50

by Dave Jones

Subject: opl3_oss use after free.

Don't read from free'd memory. Also make use of the return
value, and don't register the device if something went wrong
creating the port.

Coverity #955

Signed-off-by: Dave Jones <[email protected]>

--- linux-2.6/sound/drivers/opl3/opl3_oss.c~ 2006-03-06 03:59:35.000000000 -0500
+++ linux-2.6/sound/drivers/opl3/opl3_oss.c 2006-03-06 04:03:44.000000000 -0500
@@ -104,8 +104,10 @@ static int snd_opl3_oss_create_port(stru
voices, voices,
name);
if (opl3->oss_chset->port < 0) {
+ int port;
+ port = opl3->oss_chset->port;
snd_midi_channel_free_set(opl3->oss_chset);
- return opl3->oss_chset->port;
+ return port;
}
return 0;
}
@@ -136,10 +138,10 @@ void snd_opl3_init_seq_oss(struct snd_op
arg->oper = oss_callback;
arg->private_data = opl3;

- snd_opl3_oss_create_port(opl3);
-
- /* register to OSS synth table */
- snd_device_register(opl3->card, dev);
+ if (snd_opl3_oss_create_port(opl3)) {
+ /* register to OSS synth table */
+ snd_device_register(opl3->card, dev);
+ }
}

/* unregister */

--
http://www.codemonkey.org.uk

2006-03-06 09:10:45

by Dave Jones

Subject: Re: opl3_oss use after free.

On Mon, Mar 06, 2006 at 04:05:33AM -0500, Dave Jones wrote:
> Don't read from free'd memory. Also make use of the return
> value, and don't register the device if something went wrong
> creating the port.
>
> Coverity #955

identical bug in opl3_seq.c
This needs to check the return too, but I got lazy and just
fixed the use-after-free.

Coverity #954

Signed-off-by: Dave Jones <[email protected]>

--- linux-2.6/sound/drivers/opl3/opl3_seq.c~ 2006-03-06 04:07:42.000000000 -0500
+++ linux-2.6/sound/drivers/opl3/opl3_seq.c 2006-03-06 04:08:36.000000000 -0500
@@ -207,8 +207,10 @@ static int snd_opl3_synth_create_port(st
16, voices,
name);
if (opl3->chset->port < 0) {
+ int port;
+ port = opl3->chset->port;
snd_midi_channel_free_set(opl3->chset);
- return opl3->chset->port;
+ return port;
}
return 0;
}

--
http://www.codemonkey.org.uk

2006-03-06 13:48:09

by Takashi Iwai

Subject: Re: opl3_oss use after free.

At Mon, 6 Mar 2006 04:10:32 -0500,
Dave Jones wrote:
>
> On Mon, Mar 06, 2006 at 04:05:33AM -0500, Dave Jones wrote:
> > Don't read from free'd memory. Also make use of the return
> > value, and don't register the device if something went wrong
> > creating the port.
> >
> > Coverity #955
>
> identical bug in opl3_seq.c
> This needs to check the return too, but I got lazy and just
> fixed the use-after-free.
>
> Coverity #954
>
> Signed-off-by: Dave Jones <[email protected]>
>
> --- linux-2.6/sound/drivers/opl3/opl3_seq.c~ 2006-03-06 04:07:42.000000000 -0500
> +++ linux-2.6/sound/drivers/opl3/opl3_seq.c 2006-03-06 04:08:36.000000000 -0500
> @@ -207,8 +207,10 @@ static int snd_opl3_synth_create_port(st
> 16, voices,
> name);
> if (opl3->chset->port < 0) {
> + int port;
> + port = opl3->chset->port;
> snd_midi_channel_free_set(opl3->chset);
> - return opl3->chset->port;
> + return port;
> }
> return 0;
> }
>

I gathered both and added the proper check of return value.
Please check the below one.

Thanks.

---

[PATCH] Fix use after free in opl3_seq and opl3_oss

Don't read from free'd memory. Also make use of the return
value, and don't register the device if something went wrong
creating the port.

Coverity #954, #955

Signed-off-by: Dave Jones <[email protected]>
Signed-off-by: Takashi Iwai <[email protected]>

---
diff --git a/sound/drivers/opl3/opl3_oss.c b/sound/drivers/opl3/opl3_oss.c
index 0345ae6..fccf019 100644
--- a/sound/drivers/opl3/opl3_oss.c
+++ b/sound/drivers/opl3/opl3_oss.c
@@ -104,8 +104,10 @@ static int snd_opl3_oss_create_port(stru
voices, voices,
name);
if (opl3->oss_chset->port < 0) {
+ int port;
+ port = opl3->oss_chset->port;
snd_midi_channel_free_set(opl3->oss_chset);
- return opl3->oss_chset->port;
+ return port;
}
return 0;
}
@@ -136,10 +138,10 @@ void snd_opl3_init_seq_oss(struct snd_op
arg->oper = oss_callback;
arg->private_data = opl3;

- snd_opl3_oss_create_port(opl3);
-
- /* register to OSS synth table */
- snd_device_register(opl3->card, dev);
+ if (snd_opl3_oss_create_port(opl3)) {
+ /* register to OSS synth table */
+ snd_device_register(opl3->card, dev);
+ }
}

/* unregister */
diff --git a/sound/drivers/opl3/opl3_seq.c b/sound/drivers/opl3/opl3_seq.c
index c4ead79..56b1d1a 100644
--- a/sound/drivers/opl3/opl3_seq.c
+++ b/sound/drivers/opl3/opl3_seq.c
@@ -207,8 +207,10 @@ static int snd_opl3_synth_create_port(st
16, voices,
name);
if (opl3->chset->port < 0) {
+ int port;
+ port = opl3->chset->port;
snd_midi_channel_free_set(opl3->chset);
- return opl3->chset->port;
+ return port;
}
return 0;
}
@@ -218,7 +220,7 @@ static int snd_opl3_synth_create_port(st
static int snd_opl3_seq_new_device(struct snd_seq_device *dev)
{
struct snd_opl3 *opl3;
- int client;
+ int client, err;
char name[32];
int opl_ver;

@@ -239,7 +241,11 @@ static int snd_opl3_seq_new_device(struc
if (client < 0)
return client;

- snd_opl3_synth_create_port(opl3);
+ if ((err = snd_opl3_synth_create_port(opl3)) < 0) {
+ snd_seq_delete_kernel_client(client);
+ opl3->seq_client = -1;
+ return err;
+ }

/* initialize instrument list */
opl3->ilist = snd_seq_instr_list_new();