Re: [PATCH] Fix a use after free bug in kernel->userspace relay file support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



ACK
Thanks for catching this. Your patch looks fine. I tested for regression, no problems. I also tested the error path and had the expected results.
Thanks
Dave

Jesper Juhl wrote:
Hi,

Coverity spotted what looks like a real possible case of using a variable after it has been freed.
The problem is in kernel/relay.c::relay_open_buf()

If the code hits "goto free_buf;" it ends up in this code :

  free_buf:
    	relay_destroy_buf(buf);	<--- calls kfree() on 'buf'.
  free_name:
   	kfree(tmpname);
  end:
  	return buf;		<-- use after free of 'buf'.

I read through the callers and they all handle a NULL return from this function as an error (and hitting the 'free_buf' label only happens on failure to chan->cb->create_buf_file(), so that looks like a clear error to me). The patch simply sets 'buf' to NULL after the call to relay_destroy_buf(buf); - as far as I can see that should take care of the problem.

The patch also corrects a reference to a documentation file while I was at it.

Warning: I don't know this code at all and I've only read through it quickly to try and work out what to do here, so I'd really apreciate it if someone who actually knows how this code should behave could ACK or NACK the patch before it gets merged anywhere since I did this pretty blind.

Patch has been compile tested only.


Signed-off-by: Jesper Juhl <[email protected]>
---

diff --git a/kernel/relay.c b/kernel/relay.c
index a615a8f..c55e399 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -1,7 +1,7 @@
 /*
  * Public API and common code for kernel->userspace relay file support.
  *
- * See Documentation/filesystems/relayfs.txt for an overview of relayfs.
+ * See Documentation/filesystems/relay.txt for an overview.
  *
  * Copyright (C) 2002-2005 - Tom Zanussi ([email protected]), IBM Corp
  * Copyright (C) 1999-2005 - Karim Yaghmour ([email protected])
@@ -427,6 +427,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)

 free_buf:
  	relay_destroy_buf(buf);
+ 	buf = NULL;
 free_name:
  	kfree(tmpname);
 end:


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux