Skip to content

Commit

Permalink
USB: usbfs: properly clean up the as structure on error paths
Browse files Browse the repository at this point in the history
I notice that the processcompl_compat() function seems to be leaking the
'struct async *as' in the error paths. 

I think that the calling convention is fundamentally buggered. The
caller is the one that did the "reap_as()" to get the as thing, the
caller should be the one to free it too. 

Freeing it in the caller also means that it very clearly always gets
freed, and avoids the need for any "free in the error case too".

From: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Marcus Meissner <meissner@suse.de>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
  • Loading branch information
Linus Torvalds authored and Greg Kroah-Hartman committed Feb 16, 2010
1 parent d4a4683 commit ddeee0b
Showing 1 changed file with 26 additions and 14 deletions.
40 changes: 26 additions & 14 deletions drivers/usb/core/devio.c
Original file line number Diff line number Diff line change
Expand Up @@ -1334,14 +1334,11 @@ static int processcompl(struct async *as, void __user * __user *arg)
}
}

free_async(as);

if (put_user(addr, (void __user * __user *)arg))
return -EFAULT;
return 0;

err_out:
free_async(as);
return -EFAULT;
}

Expand Down Expand Up @@ -1371,20 +1368,28 @@ static struct async *reap_as(struct dev_state *ps)
static int proc_reapurb(struct dev_state *ps, void __user *arg)
{
struct async *as = reap_as(ps);
if (as)
return processcompl(as, (void __user * __user *)arg);
if (as) {
int retval = processcompl(as, (void __user * __user *)arg);
free_async(as);
return retval;
}
if (signal_pending(current))
return -EINTR;
return -EIO;
}

static int proc_reapurbnonblock(struct dev_state *ps, void __user *arg)
{
int retval;
struct async *as;

if (!(as = async_getcompleted(ps)))
return -EAGAIN;
return processcompl(as, (void __user * __user *)arg);
as = async_getcompleted(ps);
retval = -EAGAIN;
if (as) {
retval = processcompl(as, (void __user * __user *)arg);
free_async(as);
}
return retval;
}

#ifdef CONFIG_COMPAT
Expand Down Expand Up @@ -1497,7 +1502,6 @@ static int processcompl_compat(struct async *as, void __user * __user *arg)
}
}

free_async(as);
if (put_user(ptr_to_compat(addr), (u32 __user *)arg))
return -EFAULT;
return 0;
Expand All @@ -1506,20 +1510,28 @@ static int processcompl_compat(struct async *as, void __user * __user *arg)
static int proc_reapurb_compat(struct dev_state *ps, void __user *arg)
{
struct async *as = reap_as(ps);
if (as)
return processcompl_compat(as, (void __user * __user *)arg);
if (as) {
int retval = processcompl_compat(as, (void __user * __user *)arg);
free_async(as);
return retval;
}
if (signal_pending(current))
return -EINTR;
return -EIO;
}

static int proc_reapurbnonblock_compat(struct dev_state *ps, void __user *arg)
{
int retval;
struct async *as;

if (!(as = async_getcompleted(ps)))
return -EAGAIN;
return processcompl_compat(as, (void __user * __user *)arg);
retval = -EAGAIN;
as = async_getcompleted(ps);
if (as) {
retval = processcompl_compat(as, (void __user * __user *)arg);
free_async(as);
}
return retval;
}


Expand Down

0 comments on commit ddeee0b

Please sign in to comment.