93 lines
3.9 KiB
Diff
93 lines
3.9 KiB
Diff
From f117080307163d7057034341aa8ff6b201041599 Mon Sep 17 00:00:00 2001
|
|
From: Andrii Nakryiko <andrii@kernel.org>
|
|
Date: Mon, 10 Jul 2023 19:41:50 -0700
|
|
Subject: [PATCH] libbpf: Fix realloc API handling in zero-sized edge cases
|
|
|
|
realloc() and reallocarray() can either return NULL or a special
|
|
non-NULL pointer, if their size argument is zero. This requires a bit
|
|
more care to handle NULL-as-valid-result situation differently from
|
|
NULL-as-error case. This has caused real issues before ([0]), and just
|
|
recently bit again in production when performing bpf_program__attach_usdt().
|
|
|
|
This patch fixes 4 places that do or potentially could suffer from this
|
|
mishandling of NULL, including the reported USDT-related one.
|
|
|
|
There are many other places where realloc()/reallocarray() is used and
|
|
NULL is always treated as an error value, but all those have guarantees
|
|
that their size is always non-zero, so those spot don't need any extra
|
|
handling.
|
|
|
|
[0] d08ab82f59d5 ("libbpf: Fix double-free when linker processes empty sections")
|
|
|
|
Fixes: 999783c8bbda ("libbpf: Wire up spec management and other arch-independent USDT logic")
|
|
Fixes: b63b3c490eee ("libbpf: Add bpf_program__set_insns function")
|
|
Fixes: 697f104db8a6 ("libbpf: Support custom SEC() handlers")
|
|
Fixes: b12688267280 ("libbpf: Change the order of data and text relocations.")
|
|
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
|
|
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
|
|
Link: https://lore.kernel.org/bpf/20230711024150.1566433-1-andrii@kernel.org
|
|
---
|
|
src/libbpf.c | 15 ++++++++++++---
|
|
src/usdt.c | 5 ++++-
|
|
2 files changed, 16 insertions(+), 4 deletions(-)
|
|
|
|
diff --git a/src/libbpf.c b/src/libbpf.c
|
|
index 78635feb1..63311a73c 100644
|
|
--- a/src/libbpf.c
|
|
+++ b/src/libbpf.c
|
|
@@ -6161,7 +6161,11 @@ static int append_subprog_relos(struct bpf_program *main_prog, struct bpf_progra
|
|
if (main_prog == subprog)
|
|
return 0;
|
|
relos = libbpf_reallocarray(main_prog->reloc_desc, new_cnt, sizeof(*relos));
|
|
- if (!relos)
|
|
+ /* if new count is zero, reallocarray can return a valid NULL result;
|
|
+ * in this case the previous pointer will be freed, so we *have to*
|
|
+ * reassign old pointer to the new value (even if it's NULL)
|
|
+ */
|
|
+ if (!relos && new_cnt)
|
|
return -ENOMEM;
|
|
if (subprog->nr_reloc)
|
|
memcpy(relos + main_prog->nr_reloc, subprog->reloc_desc,
|
|
@@ -8532,7 +8536,8 @@ int bpf_program__set_insns(struct bpf_program *prog,
|
|
return -EBUSY;
|
|
|
|
insns = libbpf_reallocarray(prog->insns, new_insn_cnt, sizeof(*insns));
|
|
- if (!insns) {
|
|
+ /* NULL is a valid return from reallocarray if the new count is zero */
|
|
+ if (!insns && new_insn_cnt) {
|
|
pr_warn("prog '%s': failed to realloc prog code\n", prog->name);
|
|
return -ENOMEM;
|
|
}
|
|
@@ -8841,7 +8846,11 @@ int libbpf_unregister_prog_handler(int handler_id)
|
|
|
|
/* try to shrink the array, but it's ok if we couldn't */
|
|
sec_defs = libbpf_reallocarray(custom_sec_defs, custom_sec_def_cnt, sizeof(*sec_defs));
|
|
- if (sec_defs)
|
|
+ /* if new count is zero, reallocarray can return a valid NULL result;
|
|
+ * in this case the previous pointer will be freed, so we *have to*
|
|
+ * reassign old pointer to the new value (even if it's NULL)
|
|
+ */
|
|
+ if (sec_defs || custom_sec_def_cnt == 0)
|
|
custom_sec_defs = sec_defs;
|
|
|
|
return 0;
|
|
diff --git a/src/usdt.c b/src/usdt.c
|
|
index f1a141555..37455d00b 100644
|
|
--- a/src/usdt.c
|
|
+++ b/src/usdt.c
|
|
@@ -852,8 +852,11 @@ static int bpf_link_usdt_detach(struct bpf_link *link)
|
|
* system is so exhausted on memory, it's the least of user's
|
|
* concerns, probably.
|
|
* So just do our best here to return those IDs to usdt_manager.
|
|
+ * Another edge case when we can legitimately get NULL is when
|
|
+ * new_cnt is zero, which can happen in some edge cases, so we
|
|
+ * need to be careful about that.
|
|
*/
|
|
- if (new_free_ids) {
|
|
+ if (new_free_ids || new_cnt == 0) {
|
|
memcpy(new_free_ids + man->free_spec_cnt, usdt_link->spec_ids,
|
|
usdt_link->spec_cnt * sizeof(*usdt_link->spec_ids));
|
|
man->free_spec_ids = new_free_ids;
|
|
--
|
|
2.33.0
|