summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJesper Juhl <jj@chaosbits.net>2010-11-20 13:36:49 -0800
committerDmitry Torokhov <dmitry.torokhov@gmail.com>2010-11-20 13:38:18 -0800
commit39de52104dd92bc0548a20201350111dc9317df9 (patch)
tree610c60dac072dc641be26dc912926b8877363846
parentbc95df78c4566327086d44f1bfab984a70dc4d6b (diff)
downloadop-kernel-dev-39de52104dd92bc0548a20201350111dc9317df9.zip
op-kernel-dev-39de52104dd92bc0548a20201350111dc9317df9.tar.gz
Input: serio HIL MLC - don't deref null, don't leak and return proper error
While reviewing various users of kernel memory allocation functions I came across drivers/input/serio/hil_mlc.c::hil_mlc_register() and noticed that: - it calls kzalloc() but fails to check for a NULL return before use. - it makes several allocations and if one fails it doesn't free the previous ones. - It doesn't return -ENOMEM in the failed memory allocation case (it just crashes). This patch corrects all of the above and also reworks the only caller of this function that I could find (drivers/input/serio/hp_sdc_mlc.c::hp_sdc_mlc_out()) so that it now checks the return value of hil_mlc_register() and properly propagates it on failure and I also restructured the code to remove some labels and goto's to make it, IMHO nicer to read. Signed-off-by: Jesper Juhl <jj@chaosbits.net> Tested-by: Helge Deller <deller@gmx.de> Acked-by: Helge Deller <deller@gmx.de> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
-rw-r--r--drivers/input/serio/hil_mlc.c5
-rw-r--r--drivers/input/serio/hp_sdc_mlc.c18
2 files changed, 14 insertions, 9 deletions
diff --git a/drivers/input/serio/hil_mlc.c b/drivers/input/serio/hil_mlc.c
index e5624d8..bfd3865 100644
--- a/drivers/input/serio/hil_mlc.c
+++ b/drivers/input/serio/hil_mlc.c
@@ -932,6 +932,11 @@ int hil_mlc_register(hil_mlc *mlc)
hil_mlc_copy_di_scratch(mlc, i);
mlc_serio = kzalloc(sizeof(*mlc_serio), GFP_KERNEL);
mlc->serio[i] = mlc_serio;
+ if (!mlc->serio[i]) {
+ for (; i >= 0; i--)
+ kfree(mlc->serio[i]);
+ return -ENOMEM;
+ }
snprintf(mlc_serio->name, sizeof(mlc_serio->name)-1, "HIL_SERIO%d", i);
snprintf(mlc_serio->phys, sizeof(mlc_serio->phys)-1, "HIL%d", i);
mlc_serio->id = hil_mlc_serio_id;
diff --git a/drivers/input/serio/hp_sdc_mlc.c b/drivers/input/serio/hp_sdc_mlc.c
index 7d2b820..d50f067 100644
--- a/drivers/input/serio/hp_sdc_mlc.c
+++ b/drivers/input/serio/hp_sdc_mlc.c
@@ -305,6 +305,7 @@ static void hp_sdc_mlc_out(hil_mlc *mlc)
static int __init hp_sdc_mlc_init(void)
{
hil_mlc *mlc = &hp_sdc_mlc;
+ int err;
#ifdef __mc68000__
if (!MACH_IS_HP300)
@@ -323,22 +324,21 @@ static int __init hp_sdc_mlc_init(void)
mlc->out = &hp_sdc_mlc_out;
mlc->priv = &hp_sdc_mlc_priv;
- if (hil_mlc_register(mlc)) {
+ err = hil_mlc_register(mlc);
+ if (err) {
printk(KERN_WARNING PREFIX "Failed to register MLC structure with hil_mlc\n");
- goto err0;
+ return err;
}
if (hp_sdc_request_hil_irq(&hp_sdc_mlc_isr)) {
printk(KERN_WARNING PREFIX "Request for raw HIL ISR hook denied\n");
- goto err1;
+ if (hil_mlc_unregister(mlc))
+ printk(KERN_ERR PREFIX "Failed to unregister MLC structure with hil_mlc.\n"
+ "This is bad. Could cause an oops.\n");
+ return -EBUSY;
}
+
return 0;
- err1:
- if (hil_mlc_unregister(mlc))
- printk(KERN_ERR PREFIX "Failed to unregister MLC structure with hil_mlc.\n"
- "This is bad. Could cause an oops.\n");
- err0:
- return -EBUSY;
}
static void __exit hp_sdc_mlc_exit(void)
OpenPOWER on IntegriCloud