* [PATCH] tools: Fix use-after-free for realloc(..., 0)
@ 2022-02-12 18:18 Kees Cook
2022-02-13 1:02 ` Josh Poimboeuf
0 siblings, 1 reply; 2+ messages in thread
From: Kees Cook @ 2022-02-12 18:18 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Kees Cook, Valdis Klētnieks, Justin Forbes,
Arnaldo Carvalho de Melo, linux-kernel, linux-hardening
GCC 12 was correctly reporting a potential use-after-free condition in
the xrealloc helper. Fix the warning by avoiding an implicit "free(ptr)"
when size == 0:
In file included from help.c:12:
In function 'xrealloc',
inlined from 'add_cmdname' at help.c:24:2: subcmd-util.h:56:23: error: pointer may be used after 'realloc' [-Werror=use-after-free]
56 | ret = realloc(ptr, size);
| ^~~~~~~~~~~~~~~~~~
subcmd-util.h:52:21: note: call to 'realloc' here
52 | void *ret = realloc(ptr, size);
| ^~~~~~~~~~~~~~~~~~
subcmd-util.h:58:31: error: pointer may be used after 'realloc' [-Werror=use-after-free]
58 | ret = realloc(ptr, 1);
| ^~~~~~~~~~~~~~~
subcmd-util.h:52:21: note: call to 'realloc' here
52 | void *ret = realloc(ptr, size);
| ^~~~~~~~~~~~~~~~~~
Reported-by: "Valdis Klētnieks" <valdis.kletnieks@vt.edu>
Fixes: 2f4ce5ec1d44 ("perf tools: Finalize subcmd independence")
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
tools/lib/subcmd/subcmd-util.h | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/tools/lib/subcmd/subcmd-util.h b/tools/lib/subcmd/subcmd-util.h
index 794a375dad36..de392fc5fd3a 100644
--- a/tools/lib/subcmd/subcmd-util.h
+++ b/tools/lib/subcmd/subcmd-util.h
@@ -49,15 +49,24 @@ static NORETURN inline void die(const char *err, ...)
static inline void *xrealloc(void *ptr, size_t size)
{
- void *ret = realloc(ptr, size);
- if (!ret && !size)
- ret = realloc(ptr, 1);
+ void *ret;
+
+ /*
+ * Convert a zero-sized allocation into 1 byte, since
+ * realloc(ptr, 0) means free(ptr), but we don't want
+ * to release the memory. For a new allocation (when
+ * ptr == NULL), avoid triggering NULL-checking error
+ * conditions for zero-sized allocations.
+ */
+ if (!size)
+ size = 1;
+ ret = realloc(ptr, size);
if (!ret) {
- ret = realloc(ptr, size);
- if (!ret && !size)
- ret = realloc(ptr, 1);
- if (!ret)
- die("Out of memory, realloc failed");
+ /*
+ * If realloc() fails, the original block is left untouched;
+ * it is not freed or moved.
+ */
+ die("Out of memory, realloc failed");
}
return ret;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] tools: Fix use-after-free for realloc(..., 0)
2022-02-12 18:18 [PATCH] tools: Fix use-after-free for realloc(..., 0) Kees Cook
@ 2022-02-13 1:02 ` Josh Poimboeuf
0 siblings, 0 replies; 2+ messages in thread
From: Josh Poimboeuf @ 2022-02-13 1:02 UTC (permalink / raw)
To: Kees Cook
Cc: Valdis Klētnieks, Justin Forbes, Arnaldo Carvalho de Melo,
linux-kernel, linux-hardening
On Sat, Feb 12, 2022 at 10:18:55AM -0800, Kees Cook wrote:
> static inline void *xrealloc(void *ptr, size_t size)
> {
> - void *ret = realloc(ptr, size);
> - if (!ret && !size)
> - ret = realloc(ptr, 1);
> + void *ret;
> +
> + /*
> + * Convert a zero-sized allocation into 1 byte, since
> + * realloc(ptr, 0) means free(ptr), but we don't want
> + * to release the memory. For a new allocation (when
> + * ptr == NULL), avoid triggering NULL-checking error
> + * conditions for zero-sized allocations.
> + */
> + if (!size)
> + size = 1;
> + ret = realloc(ptr, size);
> if (!ret) {
> - ret = realloc(ptr, size);
> - if (!ret && !size)
> - ret = realloc(ptr, 1);
> - if (!ret)
> - die("Out of memory, realloc failed");
> + /*
> + * If realloc() fails, the original block is left untouched;
> + * it is not freed or moved.
> + */
> + die("Out of memory, realloc failed");
xrealloc() only has two uses -- both via ALLOC_GROW() -- and they don't
rely on the 'size == 0' freeing anyway. How about simplifying this
further to just:
ret = realloc(ptr, size);
if (!ret)
die("Out of memory, realloc failed");
Much easier to grok, and as a bonus, we don't need the long comments :-)
--
Josh
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-02-13 1:03 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-12 18:18 [PATCH] tools: Fix use-after-free for realloc(..., 0) Kees Cook
2022-02-13 1:02 ` Josh Poimboeuf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).