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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <200707182311.10538.jesper.juhl@gmail.com>
Date:	Wed, 18 Jul 2007 23:11:10 +0200
From:	Jesper Juhl <jesper.juhl@...il.com>
To:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Cc:	Tom Zanussi <zanussi@...ibm.com>,
	Karim Yaghmour <karim@...rsys.com>,
	Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>,
	Paul Mundt <lethal@...ux-sh.org>
Subject: [PATCH] Fix a use after free bug in kernel->userspace relay file support

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 <jesper.juhl@...il.com>
---

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 (zanussi@...ibm.com), IBM Corp
  * Copyright (C) 1999-2005 - Karim Yaghmour (karim@...rsys.com)
@@ -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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/