lkml.org 
[lkml]   [2010]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH RFC] serio HIL MLC: don't deref null, don't leak and return proper error
    On Sun, Nov 07, 2010 at 08:36:33PM +0100, Jesper Juhl wrote:
    > Hi,
    >
    > 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() buf 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 propagate it on
    > failure and I also restructured the code to remove some labels and goto's
    > to make it, IMHO nicer to read.
    >
    > I have no hardware to test, so please review carefully and let me know if
    > I've done something completely stupid. Please don't merge this RFC patch
    > unless at least one or more people who know this code and can actually
    > test it have ACK'ed it.

    I think Helge Deller (CCed) used to have access to such hardware...

    >
    > Ohh and please do CC me on replies.
    >
    >
    > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
    > ---
    > hil_mlc.c | 5 +++++
    > 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 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)
    >
    >
    >
    > --
    > Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
    > Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
    > Plain text mails only, please.
    >

    --
    Dmitry


    \
     
     \ /
      Last update: 2010-11-09 07:37    [W:0.029 / U:0.316 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site