Fix a use after free bug in kernel->userspace relay file support
PATCH 1/1 Fix a use after free bug in kernel->userspace relay file support
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.
Note from Mathieu: the documentation reference change should have been done in a separate patch, but I guess no one will really care.
---
kernel/relay.c +2 -1
@@ -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:
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Andrew Morton
maintainerQuoted 25 lines · On Thu, 19 Jul 2007 21:09:09 -0400 Mathieu Desnoyers…
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 tochan->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.
Quoted 7 lines
Note from Mathieu: the documentation reference change should have been done in a separate patch, but I guess no one will really care.
Signed-off-by Jesper Juhl <[email protected]>Tested-by "David J. Wilder" <[email protected]>Signed-off-by Mathieu Desnoyers <[email protected]>
I'm going to infer from all this that the patch was actually written by Jesper. Correct?
If so, it shuld have had From: Jesper Juhl <[email protected]> right at the start of the changelog.
Quoted 36 lines · * Andrew Morton ([email protected]) wrote:
Quoted 25 lines · On Thu, 19 Jul 2007 21:09:09 -0400 Mathieu Desnoyers…
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 tochan->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.
Quoted 7 lines
Note from Mathieu: the documentation reference change should have been done in a separate patch, but I guess no one will really care.
Signed-off-by Jesper Juhl <[email protected]>Tested-by "David J. Wilder" <[email protected]>Signed-off-by Mathieu Desnoyers <[email protected]>
Quoted 5 lines
I'm going to infer from all this that the patch was actually written by Jesper. Correct?
If so, it shuld have had From: Jesper Juhl <[email protected]> right at the start of the changelog.
Yes, it's the case. Thanks.
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68