From f117080307163d7057034341aa8ff6b201041599 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Mon, 10 Jul 2023 19:41:50 -0700 Subject: [PATCH 1/3] 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 Signed-off-by: Daniel Borkmann 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 78635fe..63311a7 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 f1a1415..37455d0 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; From bf88aaa6fe25aaba6f5067376eec51d452a60049 Mon Sep 17 00:00:00 2001 From: John Sanpe Date: Tue, 11 Jul 2023 15:07:12 +0800 Subject: [PATCH 2/3] libbpf: Remove HASHMAP_INIT static initialization helper Remove the wrong HASHMAP_INIT. It's not used anywhere in libbpf. Signed-off-by: John Sanpe Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/20230711070712.2064144-1-sanpeqf@gmail.com --- src/hashmap.h | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/hashmap.h b/src/hashmap.h index 0a5bf19..c12f832 100644 --- a/src/hashmap.h +++ b/src/hashmap.h @@ -80,16 +80,6 @@ struct hashmap { size_t sz; }; -#define HASHMAP_INIT(hash_fn, equal_fn, ctx) { \ - .hash_fn = (hash_fn), \ - .equal_fn = (equal_fn), \ - .ctx = (ctx), \ - .buckets = NULL, \ - .cap = 0, \ - .cap_bits = 0, \ - .sz = 0, \ -} - void hashmap__init(struct hashmap *map, hashmap_hash_fn hash_fn, hashmap_equal_fn equal_fn, void *ctx); struct hashmap *hashmap__new(hashmap_hash_fn hash_fn, From 05f94ddbb837f5f4b3161e341eed21be307eaa04 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Tue, 11 Jul 2023 09:43:03 -0700 Subject: [PATCH 3/3] sync: latest libbpf changes from kernel Syncing latest libbpf commits from kernel repository. Baseline bpf-next commit: c628747cc8800cf6d33d09f7f42c8b6f91e64dc7 Checkpoint bpf-next commit: a3e7e6b17946f48badce98d7ac360678a0ea7393 Baseline bpf commit: 496720b7cfb6574a8f6f4d434f23e3d1e6cfaeb9 Checkpoint bpf commit: 496720b7cfb6574a8f6f4d434f23e3d1e6cfaeb9 Andrii Nakryiko (1): libbpf: Fix realloc API handling in zero-sized edge cases John Sanpe (1): libbpf: Remove HASHMAP_INIT static initialization helper src/hashmap.h | 10 ---------- src/libbpf.c | 15 ++++++++++++--- src/usdt.c | 5 ++++- 3 files changed, 16 insertions(+), 14 deletions(-) Signed-off-by: Andrii Nakryiko --- CHECKPOINT-COMMIT | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHECKPOINT-COMMIT b/CHECKPOINT-COMMIT index e9557ec..130cfa6 100644 --- a/CHECKPOINT-COMMIT +++ b/CHECKPOINT-COMMIT @@ -1 +1 @@ -c628747cc8800cf6d33d09f7f42c8b6f91e64dc7 +a3e7e6b17946f48badce98d7ac360678a0ea7393