lkml.org 
[lkml]   [2011]   [Oct]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/5 v2] drivers/hid/hid-roccat.c: eliminate a null pointer dereference
    On Sat, 29 Oct 2011, Julia Lawall wrote:

    > From: Julia Lawall <julia@diku.dk>
    >
    > It is not possible to take the lock in device if device is NULL.
    > The mutex_lock is thus moved after the NULL test, and the relevant part of
    > the shared error handling code is moved up.
    >
    > The semantic match that finds this problem is as follows:
    > (http://coccinelle.lip6.fr/)
    >
    > // <smpl>
    > @r@
    > expression E, E1;
    > identifier f;
    > statement S1,S2,S3;
    > @@
    >
    > if (E == NULL)
    > {
    > ... when != if (E == NULL || ...) S1 else S2
    > when != E = E1
    > *E->f
    > ... when any
    > return ...;
    > }
    > else S3
    > // </smpl>
    >
    > Signed-off-by: Julia Lawall <julia@diku.dk>
    >
    > ---
    > mutex_lock changed to mutex_unlock in error handling code
    >
    > drivers/hid/hid-roccat.c | 9 +++++----
    > 1 file changed, 5 insertions(+), 4 deletions(-)
    >
    > diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
    > index 2596321..36a28b8 100644
    > --- a/drivers/hid/hid-roccat.c
    > +++ b/drivers/hid/hid-roccat.c
    > @@ -163,14 +163,15 @@ static int roccat_open(struct inode *inode, struct file *file)
    >
    > device = devices[minor];
    >
    > - mutex_lock(&device->readers_lock);
    > -
    > if (!device) {
    > pr_emerg("roccat device with minor %d doesn't exist\n", minor);
    > - error = -ENODEV;
    > - goto exit_err;
    > + kfree(reader);
    > + mutex_unlock(&devices_lock);
    > + return -ENODEV;
    > }
    >
    > + mutex_lock(&device->readers_lock);
    > +
    > if (!device->open++) {
    > /* power on device on adding first reader */
    > error = hid_hw_power(device->hid, PM_HINT_FULLON);

    Julia,

    thanks a lot for fixing this.

    Could you please redo the patch in a way that it adds second
    'exit_unlock1' label (and renames 'exit_unlock' to 'exit_unlock2') (or
    any appropriate variation of the names) and preserve error path using goto
    instead of mixture of returns and gotos that this patch would introduce?

    Thanks,

    --
    Jiri Kosina
    SUSE Labs


    \
     
     \ /
      Last update: 2011-10-29 19:03    [W:0.028 / U:0.016 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site