Hi,
Fix possible null pointer kfree.
Signed-off-by: dave young <[email protected]>
mixer_oss.c | 120 +-
1 file changed, 74 insertions(+), 46 deletions(-)
diff -purN linux/sound/core/oss/mixer_oss.c linux.new/sound/core/oss/mixer_oss.c
--- linux/sound/core/oss/mixer_oss.c 2007-06-01 09:00:12.000000000 +0000
+++ linux.new/sound/core/oss/mixer_oss.c 2007-06-01 09:36:10.000000000 +0000
@@ -512,23 +512,27 @@ static void snd_mixer_oss_get_volume1_vo
return;
}
uinfo = kzalloc(sizeof(*uinfo), GFP_KERNEL);
+ if (uinfo == NULL)
+ goto out_uinfo;
uctl = kzalloc(sizeof(*uctl), GFP_KERNEL);
- if (uinfo == NULL || uctl == NULL)
- goto __unalloc;
+ if (uctl == NULL)
+ goto out_uctl;
if (kctl->info(kctl, uinfo))
- goto __unalloc;
+ goto out;
if (kctl->get(kctl, uctl))
- goto __unalloc;
+ goto out;
if (uinfo->type == SNDRV_CTL_ELEM_TYPE_BOOLEAN &&
uinfo->value.integer.min == 0 && uinfo->value.integer.max == 1)
- goto __unalloc;
+ goto out;
*left = snd_mixer_oss_conv1(uctl->value.integer.value[0],
uinfo->value.integer.min, uinfo->value.integer.max,
&pslot->volume[0]);
if (uinfo->count > 1)
*right = snd_mixer_oss_conv1(uctl->value.integer.value[1],
uinfo->value.integer.min, uinfo->value.integer.max,
&pslot->volume[1]);
- __unalloc:
+out:
+ kfree(uctl);
+out_uctl:
+ kfree(uinfo);
+out_uinfo:
up_read(&card->controls_rwsem);
- kfree(uctl);
- kfree(uinfo);
}
static void snd_mixer_oss_get_volume1_sw(struct snd_mixer_oss_file *fmixer,
@@ -550,13 +554,15 @@ static void snd_mixer_oss_get_volume1_sw
return;
}
uinfo = kzalloc(sizeof(*uinfo), GFP_KERNEL);
+ if (uinfo == NULL)
+ goto out_uinfo;
uctl = kzalloc(sizeof(*uctl), GFP_KERNEL);
- if (uinfo == NULL || uctl == NULL)
- goto __unalloc;
+ if (uctl == NULL)
+ goto out_uctl;
if (kctl->info(kctl, uinfo))
- goto __unalloc;
+ goto out;
if (kctl->get(kctl, uctl))
- goto __unalloc;
+ goto out;
if (!uctl->value.integer.value[0]) {
*left = 0;
if (uinfo->count == 1)
@@ -564,10 +570,12 @@ static void snd_mixer_oss_get_volume1_sw
}
if (uinfo->count > 1 && !uctl->value.integer.value[route ? 3 : 1])
*right = 0;
- __unalloc:
- up_read(&card->controls_rwsem);
- kfree(uctl);
+out:
+ kfree(uctl);
+out_uctl:
kfree(uinfo);
+out_uinfo:
+ up_read(&card->controls_rwsem);
}
static int snd_mixer_oss_get_volume1(struct snd_mixer_oss_file *fmixer,
@@ -613,25 +621,29 @@ static void snd_mixer_oss_put_volume1_vo
if ((kctl = snd_ctl_find_numid(card, numid)) == NULL)
return;
uinfo = kzalloc(sizeof(*uinfo), GFP_KERNEL);
+ if (uinfo == NULL)
+ goto out_uinfo;
uctl = kzalloc(sizeof(*uctl), GFP_KERNEL);
- if (uinfo == NULL || uctl == NULL)
- goto __unalloc;
+ if (uctl == NULL)
+ goto out_uctl;
if (kctl->info(kctl, uinfo))
- goto __unalloc;
+ goto out;
if (uinfo->type == SNDRV_CTL_ELEM_TYPE_BOOLEAN &&
uinfo->value.integer.min == 0 && uinfo->value.integer.max == 1)
- goto __unalloc;
+ goto out;
uctl->value.integer.value[0] = snd_mixer_oss_conv2(left,
uinfo->value.integer.min, uinfo->value.integer.max);
if (uinfo->count > 1)
uctl->value.integer.value[1] = snd_mixer_oss_conv2(right,
uinfo->value.integer.min, uinfo->value.integer.max);
if ((res = kctl->put(kctl, uctl)) < 0)
- goto __unalloc;
+ goto out;
if (res > 0)
snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_VALUE, &kctl->id);
- __unalloc:
- up_read(&card->controls_rwsem);
- kfree(uctl);
+out:
+ kfree(uctl);
+out_uctl:
kfree(uinfo);
+out_uinfo:
+ up_read(&card->controls_rwsem);
}
static void snd_mixer_oss_put_volume1_sw(struct snd_mixer_oss_file *fmixer,
@@ -654,11 +666,13 @@ static void snd_mixer_oss_put_volume1_sw
return;
}
uinfo = kzalloc(sizeof(*uinfo), GFP_KERNEL);
+ if (uinfo == NULL)
+ goto out_uinfo;
uctl = kzalloc(sizeof(*uctl), GFP_KERNEL);
- if (uinfo == NULL || uctl == NULL)
- goto __unalloc;
+ if (uctl == NULL)
+ goto out_uctl;
if (kctl->info(kctl, uinfo))
- goto __unalloc;
+ goto out;
if (uinfo->count > 1) {
uctl->value.integer.value[0] = left > 0 ? 1 : 0;
uctl->value.integer.value[route ? 3 : 1] = right > 0 ? 1 : 0;
@@ -670,13 +684,15 @@ static void snd_mixer_oss_put_volume1_sw
uctl->value.integer.value[0] = (left > 0 || right > 0) ? 1 : 0;
}
if ((res = kctl->put(kctl, uctl)) < 0)
- goto __unalloc;
+ goto out;
if (res > 0)
snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_VALUE, &kctl->id);
- __unalloc:
- up_read(&card->controls_rwsem);
- kfree(uctl);
+out:
+ kfree(uctl);
+out_uctl:
kfree(uinfo);
+out_uinfo:
+ up_read(&card->controls_rwsem);
}
static int snd_mixer_oss_put_volume1(struct snd_mixer_oss_file *fmixer,
@@ -775,21 +791,25 @@ static int snd_mixer_oss_get_recsrc2(str
int err, idx;
uinfo = kzalloc(sizeof(*uinfo), GFP_KERNEL);
+ if (uinfo == NULL){
+ err = -ENOMEM;
+ goto out_uinfo;
+ }
uctl = kzalloc(sizeof(*uctl), GFP_KERNEL);
- if (uinfo == NULL || uctl == NULL) {
+ if (uctl == NULL) {
err = -ENOMEM;
- goto __unlock;
+ goto out_uctl;
}
down_read(&card->controls_rwsem);
kctl = snd_mixer_oss_test_id(mixer, "Capture Source", 0);
if (! kctl) {
err = -ENOENT;
- goto __unlock;
+ goto out;
}
if ((err = kctl->info(kctl, uinfo)) < 0)
- goto __unlock;
+ goto out;
if ((err = kctl->get(kctl, uctl)) < 0)
- goto __unlock;
+ goto out;
for (idx = 0; idx < 32; idx++) {
if (!(mixer->mask_recsrc & (1 << idx)))
continue;
@@ -805,10 +825,12 @@ static int snd_mixer_oss_get_recsrc2(str
}
}
err = 0;
- __unlock:
+out:
+ kfree(uctl);
+out_uctl:
+ kfree(uinfo);
+out_uinfo:
up_read(&card->controls_rwsem);
- kfree(uctl);
- kfree(uinfo);
return err;
}
@@ -825,19 +847,23 @@ static int snd_mixer_oss_put_recsrc2(str
unsigned int idx;
uinfo = kzalloc(sizeof(*uinfo), GFP_KERNEL);
+ if (uinfo == NULL){
+ err = -ENOMEM;
+ goto out_uinfo;
+ }
uctl = kzalloc(sizeof(*uctl), GFP_KERNEL);
- if (uinfo == NULL || uctl == NULL) {
+ if (uctl == NULL) {
err = -ENOMEM;
- goto __unlock;
+ goto out_uctl;
}
down_read(&card->controls_rwsem);
kctl = snd_mixer_oss_test_id(mixer, "Capture Source", 0);
if (! kctl) {
err = -ENOENT;
- goto __unlock;
+ goto out;
}
if ((err = kctl->info(kctl, uinfo)) < 0)
- goto __unlock;
+ goto out;
for (idx = 0; idx < 32; idx++) {
if (!(mixer->mask_recsrc & (1 << idx)))
continue;
@@ -852,18 +878,20 @@ static int snd_mixer_oss_put_recsrc2(str
slot = NULL;
}
if (! slot)
- goto __unlock;
+ goto out;
for (idx = 0; idx < uinfo->count; idx++)
uctl->value.enumerated.item[idx] = slot->capture_item;
err = kctl->put(kctl, uctl);
if (err > 0)
snd_ctl_notify(fmixer->card, SNDRV_CTL_EVENT_MASK_VALUE, &kctl->id);
err = 0;
- __unlock:
- up_read(&card->controls_rwsem);
+out:
kfree(uctl);
+out_uctl:
kfree(uinfo);
- return err;
+out_uinfo:
+ up_read(&card->controls_rwsem);
+ return err;
}
struct snd_mixer_oss_assign_table {
Regards
dave
On Fri, 1 Jun 2007 01:53:39 +0000 "young dave" <[email protected]> wrote:
> Fix possible null pointer kfree.
kfree(NULL) is legal, and is often used.
Hi,
> kfree(NULL) is legal, and is often used.
Really? I don't know this before.
Regards
dave
Hi,
Just append to my email.
The reason I read mixer_oss.c is that I see oops in 2.6.22-rc1-mm1,
seems oops in mixer_oss.c, but the oops rised only once, I found
rc3-mm1 have no change in this file,
so I read the source , then post this patch.
please find the oops message below:
BUG: unable to handle kernel NULL pointer dereference at virtual
address 00000088
printing eip:
c0162bc2
*pde = 00000000
Oops: 0000 [#1]
PREEMPT
Modules linked in: snd_mixer_oss snd_hda_intel snd_pcm snd_timer
snd_page_alloc snd soundcore ipv6 capability commoncap e100 mi
i agpgart pcspkr psmouse
CPU: 0
EIP: 0060:[<c0162bc2>] Not tainted VLI
EFLAGS: 00010087 (2.6.22-rc1-mm1 #7)
EIP is at kfree+0x42/0x90
eax: 00000000 ebx: c10568e0 ecx: c2b47000 edx: c01588d2
esi: 00000287 edi: 00000000 ebp: c2864000 esp: c2865f58
ds: 007b es: 007b fs: 0000 gs: 0033 ss: 0068
Process modprobe (pid: 2963, ti=c2864000 task=c1ede540 task.ti=c2864000)
Stack: 00000011 f8855000 c0520cb4 00000001 00000001 c1e524a0 c1051bc0 c01588d2
0805c348 00000001 c2864000 f885502b f8840a00 00000000 00000001 c013caf1
00004b79 00000000 000b7f68 00000000 b7f44840 08065f88 0805c348 c01040bc
Call Trace:
[<f8855000>] alsa_mixer_oss_init+0x0/0x37 [snd_mixer_oss]
[<c01588d2>] __vunmap+0xb2/0xe0
[<f885502b>] alsa_mixer_oss_init+0x2b/0x37 [snd_mixer_oss]
[<c013caf1>] sys_init_module+0xa1/0x140
[<c01040bc>] syscall_call+0x7/0xb
[<c0440000>] packet_seq_start+0x30/0x60
=======================
Code: 00 00 00 40 a1 40 6a 59 c0 c1 ea 0c c1 e2 05 01 c2 8b 02 89 d3
25 00 40 02 00 3d 00 40 02 00 74 45 8b 7b 10 8b 54 24 1c 9
c 5e fa <39> 9f 88 00 00 00 75 25 8b 03 a8 02 75 1f 0f b7 53 0a 8b 43 0c
EIP: [<c0162bc2>] kfree+0x42/0x90 SS:ESP 0068:c2865f58
Hi, Andrew
> kfree(NULL) is legal, and is often used.
>
Apart from the null pointer, IMHO,there's two problem need to be
fixed, I'm not sure.
What's your opinion?
1. the label indent should be removed
2. similar code use different label, some use " __unlock" but others
use "__unalloc", "__unlock" seems to be misspelled.
Regards
dave