diff options
author | Jesper Juhl <jj@chaosbits.net> | 2010-11-20 16:36:49 -0500 |
---|---|---|
committer | Dmitry Torokhov <dmitry.torokhov@gmail.com> | 2010-11-20 16:38:18 -0500 |
commit | 39de52104dd92bc0548a20201350111dc9317df9 (patch) | |
tree | 610c60dac072dc641be26dc912926b8877363846 /drivers/input | |
parent | bc95df78c4566327086d44f1bfab984a70dc4d6b (diff) |
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>
Diffstat (limited to 'drivers/input')
-rw-r--r-- | drivers/input/serio/hil_mlc.c | 5 | ||||
-rw-r--r-- | drivers/input/serio/hp_sdc_mlc.c | 18 |
2 files changed, 14 insertions, 9 deletions
diff --git a/drivers/input/serio/hil_mlc.c b/drivers/input/serio/hil_mlc.c index e5624d8f1709..bfd3865d886b 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) | |||
932 | hil_mlc_copy_di_scratch(mlc, i); | 932 | hil_mlc_copy_di_scratch(mlc, i); |
933 | mlc_serio = kzalloc(sizeof(*mlc_serio), GFP_KERNEL); | 933 | mlc_serio = kzalloc(sizeof(*mlc_serio), GFP_KERNEL); |
934 | mlc->serio[i] = mlc_serio; | 934 | mlc->serio[i] = mlc_serio; |
935 | if (!mlc->serio[i]) { | ||
936 | for (; i >= 0; i--) | ||
937 | kfree(mlc->serio[i]); | ||
938 | return -ENOMEM; | ||
939 | } | ||
935 | snprintf(mlc_serio->name, sizeof(mlc_serio->name)-1, "HIL_SERIO%d", i); | 940 | snprintf(mlc_serio->name, sizeof(mlc_serio->name)-1, "HIL_SERIO%d", i); |
936 | snprintf(mlc_serio->phys, sizeof(mlc_serio->phys)-1, "HIL%d", i); | 941 | snprintf(mlc_serio->phys, sizeof(mlc_serio->phys)-1, "HIL%d", i); |
937 | mlc_serio->id = hil_mlc_serio_id; | 942 | 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 7d2b820ef58d..d50f0678bf47 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) | |||
305 | static int __init hp_sdc_mlc_init(void) | 305 | static int __init hp_sdc_mlc_init(void) |
306 | { | 306 | { |
307 | hil_mlc *mlc = &hp_sdc_mlc; | 307 | hil_mlc *mlc = &hp_sdc_mlc; |
308 | int err; | ||
308 | 309 | ||
309 | #ifdef __mc68000__ | 310 | #ifdef __mc68000__ |
310 | if (!MACH_IS_HP300) | 311 | if (!MACH_IS_HP300) |
@@ -323,22 +324,21 @@ static int __init hp_sdc_mlc_init(void) | |||
323 | mlc->out = &hp_sdc_mlc_out; | 324 | mlc->out = &hp_sdc_mlc_out; |
324 | mlc->priv = &hp_sdc_mlc_priv; | 325 | mlc->priv = &hp_sdc_mlc_priv; |
325 | 326 | ||
326 | if (hil_mlc_register(mlc)) { | 327 | err = hil_mlc_register(mlc); |
328 | if (err) { | ||
327 | printk(KERN_WARNING PREFIX "Failed to register MLC structure with hil_mlc\n"); | 329 | printk(KERN_WARNING PREFIX "Failed to register MLC structure with hil_mlc\n"); |
328 | goto err0; | 330 | return err; |
329 | } | 331 | } |
330 | 332 | ||
331 | if (hp_sdc_request_hil_irq(&hp_sdc_mlc_isr)) { | 333 | if (hp_sdc_request_hil_irq(&hp_sdc_mlc_isr)) { |
332 | printk(KERN_WARNING PREFIX "Request for raw HIL ISR hook denied\n"); | 334 | printk(KERN_WARNING PREFIX "Request for raw HIL ISR hook denied\n"); |
333 | goto err1; | 335 | if (hil_mlc_unregister(mlc)) |
336 | printk(KERN_ERR PREFIX "Failed to unregister MLC structure with hil_mlc.\n" | ||
337 | "This is bad. Could cause an oops.\n"); | ||
338 | return -EBUSY; | ||
334 | } | 339 | } |
340 | |||
335 | return 0; | 341 | return 0; |
336 | err1: | ||
337 | if (hil_mlc_unregister(mlc)) | ||
338 | printk(KERN_ERR PREFIX "Failed to unregister MLC structure with hil_mlc.\n" | ||
339 | "This is bad. Could cause an oops.\n"); | ||
340 | err0: | ||
341 | return -EBUSY; | ||
342 | } | 342 | } |
343 | 343 | ||
344 | static void __exit hp_sdc_mlc_exit(void) | 344 | static void __exit hp_sdc_mlc_exit(void) |