opl3_oss use after free.

opl3_oss use after free.

[Dave Jones]

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 <davej@redhat.com>

--- 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

Re: opl3_oss use after free.

[Dave Jones]

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 <davej@redhat.com>

--- 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

Re: opl3_oss use after free.

[Takashi Iwai]

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 <davej@redhat.com>
> 
> --- 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 <davej@redhat.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>

---
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();