* [PATCH v2 bpf-next 0/5] libbpf ELF sanity checking improvements
@ 2021-11-03 17:32 Andrii Nakryiko
2021-11-03 17:32 ` [PATCH v2 bpf-next 1/5] libbpf: detect corrupted ELF symbols section Andrii Nakryiko
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2021-11-03 17:32 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team
Few patches fixing various issues discovered by oss-fuzz project fuzzing
bpf_object__open() call. Fixes are mostly focused around additional simple
sanity checks of ELF format: symbols, relos, etc.
v1->v2:
- address Yonghong's feedback.
Andrii Nakryiko (5):
libbpf: detect corrupted ELF symbols section
libbpf: improve sanity checking during BTF fix up
libbpf: validate that .BTF and .BTF.ext sections contain data
libbpf: fix section counting logic
libbpf: improve ELF relo sanitization
tools/lib/bpf/libbpf.c | 43 +++++++++++++++++++++++++++++-------------
1 file changed, 30 insertions(+), 13 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 bpf-next 1/5] libbpf: detect corrupted ELF symbols section
2021-11-03 17:32 [PATCH v2 bpf-next 0/5] libbpf ELF sanity checking improvements Andrii Nakryiko
@ 2021-11-03 17:32 ` Andrii Nakryiko
2021-11-03 17:32 ` [PATCH v2 bpf-next 2/5] libbpf: improve sanity checking during BTF fix up Andrii Nakryiko
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2021-11-03 17:32 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team, Yonghong Song
Prevent divide-by-zero if ELF is corrupted and has zero sh_entsize.
Reported by oss-fuzz project.
Acked-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/libbpf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index a1bea1953df6..71f5a009010a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3555,7 +3555,7 @@ static int bpf_object__collect_externs(struct bpf_object *obj)
scn = elf_sec_by_idx(obj, obj->efile.symbols_shndx);
sh = elf_sec_hdr(obj, scn);
- if (!sh)
+ if (!sh || sh->sh_entsize != sizeof(Elf64_Sym))
return -LIBBPF_ERRNO__FORMAT;
dummy_var_btf_id = add_dummy_ksym_var(obj->btf);
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 bpf-next 2/5] libbpf: improve sanity checking during BTF fix up
2021-11-03 17:32 [PATCH v2 bpf-next 0/5] libbpf ELF sanity checking improvements Andrii Nakryiko
2021-11-03 17:32 ` [PATCH v2 bpf-next 1/5] libbpf: detect corrupted ELF symbols section Andrii Nakryiko
@ 2021-11-03 17:32 ` Andrii Nakryiko
2021-11-03 17:32 ` [PATCH v2 bpf-next 3/5] libbpf: validate that .BTF and .BTF.ext sections contain data Andrii Nakryiko
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2021-11-03 17:32 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team, Yonghong Song
If BTF is corrupted DATASEC's variable type ID might be incorrect.
Prevent this easy to detect situation with extra NULL check.
Reported by oss-fuzz project.
Acked-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/libbpf.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 71f5a009010a..f836a1936597 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2752,13 +2752,12 @@ static int btf_fixup_datasec(struct bpf_object *obj, struct btf *btf,
for (i = 0, vsi = btf_var_secinfos(t); i < vars; i++, vsi++) {
t_var = btf__type_by_id(btf, vsi->type);
- var = btf_var(t_var);
-
- if (!btf_is_var(t_var)) {
+ if (!t_var || !btf_is_var(t_var)) {
pr_debug("Non-VAR type seen in section %s\n", name);
return -EINVAL;
}
+ var = btf_var(t_var);
if (var->linkage == BTF_VAR_STATIC)
continue;
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 bpf-next 3/5] libbpf: validate that .BTF and .BTF.ext sections contain data
2021-11-03 17:32 [PATCH v2 bpf-next 0/5] libbpf ELF sanity checking improvements Andrii Nakryiko
2021-11-03 17:32 ` [PATCH v2 bpf-next 1/5] libbpf: detect corrupted ELF symbols section Andrii Nakryiko
2021-11-03 17:32 ` [PATCH v2 bpf-next 2/5] libbpf: improve sanity checking during BTF fix up Andrii Nakryiko
@ 2021-11-03 17:32 ` Andrii Nakryiko
2021-11-03 17:32 ` [PATCH v2 bpf-next 4/5] libbpf: fix section counting logic Andrii Nakryiko
2021-11-03 17:32 ` [PATCH v2 bpf-next 5/5] libbpf: improve ELF relo sanitization Andrii Nakryiko
4 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2021-11-03 17:32 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team, Yonghong Song
.BTF and .BTF.ext ELF sections should have SHT_PROGBITS type and contain
data. If they are not, ELF is invalid or corrupted, so bail out.
Otherwise this can lead to data->d_buf being NULL and SIGSEGV later on.
Reported by oss-fuzz project.
Acked-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/libbpf.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index f836a1936597..0dc6465271ce 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3270,8 +3270,12 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
} else if (strcmp(name, MAPS_ELF_SEC) == 0) {
obj->efile.btf_maps_shndx = idx;
} else if (strcmp(name, BTF_ELF_SEC) == 0) {
+ if (sh->sh_type != SHT_PROGBITS)
+ return -LIBBPF_ERRNO__FORMAT;
btf_data = data;
} else if (strcmp(name, BTF_EXT_ELF_SEC) == 0) {
+ if (sh->sh_type != SHT_PROGBITS)
+ return -LIBBPF_ERRNO__FORMAT;
btf_ext_data = data;
} else if (sh->sh_type == SHT_SYMTAB) {
/* already processed during the first pass above */
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 bpf-next 4/5] libbpf: fix section counting logic
2021-11-03 17:32 [PATCH v2 bpf-next 0/5] libbpf ELF sanity checking improvements Andrii Nakryiko
` (2 preceding siblings ...)
2021-11-03 17:32 ` [PATCH v2 bpf-next 3/5] libbpf: validate that .BTF and .BTF.ext sections contain data Andrii Nakryiko
@ 2021-11-03 17:32 ` Andrii Nakryiko
2021-11-03 17:32 ` [PATCH v2 bpf-next 5/5] libbpf: improve ELF relo sanitization Andrii Nakryiko
4 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2021-11-03 17:32 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team, Yonghong Song
e_shnum does include section #0 and as such is exactly the number of ELF
sections that we need to allocate memory for to use section indices as
array indices. Fix the off-by-one error.
This is purely accounting fix, previously we were overallocating one
too many array items. But no correctness errors otherwise.
Fixes: 25bbbd7a444b ("libbpf: Remove assumptions about uniqueness of .rodata/.data/.bss maps")
Acked-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/libbpf.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 0dc6465271ce..ecfea6c20042 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3190,11 +3190,11 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
Elf_Scn *scn;
Elf64_Shdr *sh;
- /* ELF section indices are 1-based, so allocate +1 element to keep
- * indexing simple. Also include 0th invalid section into sec_cnt for
- * simpler and more traditional iteration logic.
+ /* ELF section indices are 0-based, but sec #0 is special "invalid"
+ * section. e_shnum does include sec #0, so e_shnum is the necessary
+ * size of an array to keep all the sections.
*/
- obj->efile.sec_cnt = 1 + obj->efile.ehdr->e_shnum;
+ obj->efile.sec_cnt = obj->efile.ehdr->e_shnum;
obj->efile.secs = calloc(obj->efile.sec_cnt, sizeof(*obj->efile.secs));
if (!obj->efile.secs)
return -ENOMEM;
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 bpf-next 5/5] libbpf: improve ELF relo sanitization
2021-11-03 17:32 [PATCH v2 bpf-next 0/5] libbpf ELF sanity checking improvements Andrii Nakryiko
` (3 preceding siblings ...)
2021-11-03 17:32 ` [PATCH v2 bpf-next 4/5] libbpf: fix section counting logic Andrii Nakryiko
@ 2021-11-03 17:32 ` Andrii Nakryiko
4 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2021-11-03 17:32 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team, Yonghong Song
Add few sanity checks for relocations to prevent div-by-zero and
out-of-bounds array accesses in libbpf.
Acked-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/libbpf.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ecfea6c20042..86a44735230e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3306,6 +3306,10 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
} else if (sh->sh_type == SHT_REL) {
int targ_sec_idx = sh->sh_info; /* points to other section */
+ if (sh->sh_entsize != sizeof(Elf64_Rel) ||
+ targ_sec_idx >= obj->efile.sec_cnt)
+ return -LIBBPF_ERRNO__FORMAT;
+
/* Only do relo for section with exec instructions */
if (!section_have_execinstr(obj, targ_sec_idx) &&
strcmp(name, ".rel" STRUCT_OPS_SEC) &&
@@ -4025,7 +4029,7 @@ static int
bpf_object__collect_prog_relos(struct bpf_object *obj, Elf64_Shdr *shdr, Elf_Data *data)
{
const char *relo_sec_name, *sec_name;
- size_t sec_idx = shdr->sh_info;
+ size_t sec_idx = shdr->sh_info, sym_idx;
struct bpf_program *prog;
struct reloc_desc *relos;
int err, i, nrels;
@@ -4036,6 +4040,9 @@ bpf_object__collect_prog_relos(struct bpf_object *obj, Elf64_Shdr *shdr, Elf_Dat
Elf64_Sym *sym;
Elf64_Rel *rel;
+ if (sec_idx >= obj->efile.sec_cnt)
+ return -EINVAL;
+
scn = elf_sec_by_idx(obj, sec_idx);
scn_data = elf_sec_data(obj, scn);
@@ -4055,16 +4062,23 @@ bpf_object__collect_prog_relos(struct bpf_object *obj, Elf64_Shdr *shdr, Elf_Dat
return -LIBBPF_ERRNO__FORMAT;
}
- sym = elf_sym_by_idx(obj, ELF64_R_SYM(rel->r_info));
+ sym_idx = ELF64_R_SYM(rel->r_info);
+ sym = elf_sym_by_idx(obj, sym_idx);
if (!sym) {
- pr_warn("sec '%s': symbol 0x%zx not found for relo #%d\n",
- relo_sec_name, (size_t)ELF64_R_SYM(rel->r_info), i);
+ pr_warn("sec '%s': symbol #%zu not found for relo #%d\n",
+ relo_sec_name, sym_idx, i);
+ return -LIBBPF_ERRNO__FORMAT;
+ }
+
+ if (sym->st_shndx >= obj->efile.sec_cnt) {
+ pr_warn("sec '%s': corrupted symbol #%zu pointing to invalid section #%zu for relo #%d\n",
+ relo_sec_name, sym_idx, (size_t)sym->st_shndx, i);
return -LIBBPF_ERRNO__FORMAT;
}
if (rel->r_offset % BPF_INSN_SZ || rel->r_offset >= scn_data->d_size) {
pr_warn("sec '%s': invalid offset 0x%zx for relo #%d\n",
- relo_sec_name, (size_t)ELF64_R_SYM(rel->r_info), i);
+ relo_sec_name, (size_t)rel->r_offset, i);
return -LIBBPF_ERRNO__FORMAT;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-03 17:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-03 17:32 [PATCH v2 bpf-next 0/5] libbpf ELF sanity checking improvements Andrii Nakryiko
2021-11-03 17:32 ` [PATCH v2 bpf-next 1/5] libbpf: detect corrupted ELF symbols section Andrii Nakryiko
2021-11-03 17:32 ` [PATCH v2 bpf-next 2/5] libbpf: improve sanity checking during BTF fix up Andrii Nakryiko
2021-11-03 17:32 ` [PATCH v2 bpf-next 3/5] libbpf: validate that .BTF and .BTF.ext sections contain data Andrii Nakryiko
2021-11-03 17:32 ` [PATCH v2 bpf-next 4/5] libbpf: fix section counting logic Andrii Nakryiko
2021-11-03 17:32 ` [PATCH v2 bpf-next 5/5] libbpf: improve ELF relo sanitization Andrii Nakryiko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.