[PATCH bpf-next v4 0/2] bpf: expand bpf_d_path helper allowlist

[PATCH bpf-next v4 0/2] bpf: expand bpf_d_path helper allowlist

[Hengqi Chen]

This patch set adds more functions to bpf_d_path allowlist.

Patch 1 is prep work which updates resolve_btfids to emit warnings
on missing symbols instead of aborting kernel build process.

Patch 2 expands bpf_d_path allowlist.

Changes since v3: [3]
 - Addressed Yonghong's comments. Sort allowlist and add security_bprm_*

Changes since v2: [2]
 - Andrii suggested that we should first address an issue of .BTF_ids
   before adding more symbols to .BTF_ids. Fixed that.
 - Yaniv proposed adding security_sb_mount and security_bprm_check.
   Added them.

Changes since v1: [1]
 - Alexei and Yonghong suggested that bpf_d_path helper could also
   apply to vfs_* and security_file_* kernel functions. Added them.

[1] https://lore.kernel.org/bpf/20210712162424.2034006-1-hengqi.chen@gmail.com/
[2] https://lore.kernel.org/bpf/20210719151753.399227-1-hengqi.chen@gmail.com/
[3] https://lore.kernel.org/bpf/20210725141814.2000828-3-hengqi.chen@gmail.com/

Hengqi Chen (2):
  tools/resolve_btfids: emit warnings and patch zero id for missing
    symbols
  bpf: expose bpf_d_path helper to vfs_* and security_* functions

 kernel/trace/bpf_trace.c        | 60 ++++++++++++++++++++++++++++++---
 tools/bpf/resolve_btfids/main.c | 13 +++----
 2 files changed, 63 insertions(+), 10 deletions(-)

-- 
2.25.1

[PATCH bpf-next v4 1/2] tools/resolve_btfids: emit warnings and patch zero id for missing symbols

[Hengqi Chen]

Kernel functions referenced by .BTF_ids may be changed from global to static
and get inlined or get renamed/removed, and thus disappears from BTF.
This causes kernel build failure when resolve_btfids do id patch for symbols
in .BTF_ids in vmlinux. Update resolve_btfids to emit warning messages and
patch zero id for missing symbols instead of aborting kernel build process.

Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
 tools/bpf/resolve_btfids/main.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
index 3ad9301b0f00..3ea19e33250d 100644
--- a/tools/bpf/resolve_btfids/main.c
+++ b/tools/bpf/resolve_btfids/main.c
@@ -291,7 +291,7 @@ static int compressed_section_fix(Elf *elf, Elf_Scn *scn, GElf_Shdr *sh)
 	sh->sh_addralign = expected;
 
 	if (gelf_update_shdr(scn, sh) == 0) {
-		printf("FAILED cannot update section header: %s\n",
+		pr_err("FAILED cannot update section header: %s\n",
 			elf_errmsg(-1));
 		return -1;
 	}
@@ -317,6 +317,7 @@ static int elf_collect(struct object *obj)
 
 	elf = elf_begin(fd, ELF_C_RDWR_MMAP, NULL);
 	if (!elf) {
+		close(fd);
 		pr_err("FAILED cannot create ELF descriptor: %s\n",
 			elf_errmsg(-1));
 		return -1;
@@ -484,7 +485,7 @@ static int symbols_resolve(struct object *obj)
 	err = libbpf_get_error(btf);
 	if (err) {
 		pr_err("FAILED: load BTF from %s: %s\n",
-			obj->path, strerror(-err));
+			obj->btf ?: obj->path, strerror(-err));
 		return -1;
 	}
 
@@ -555,8 +556,7 @@ static int id_patch(struct object *obj, struct btf_id *id)
 	int i;
 
 	if (!id->id) {
-		pr_err("FAILED unresolved symbol %s\n", id->name);
-		return -EINVAL;
+		pr_err("WARN: unresolved symbol %s\n", id->name);
 	}
 
 	for (i = 0; i < id->addr_cnt; i++) {
@@ -734,8 +734,9 @@ int main(int argc, const char **argv)
 
 	err = 0;
 out:
-	if (obj.efile.elf)
+	if (obj.efile.elf) {
 		elf_end(obj.efile.elf);
-	close(obj.efile.fd);
+		close(obj.efile.fd);
+	}
 	return err;
 }
-- 
2.25.1

[PATCH bpf-next v4 2/2] bpf: expose bpf_d_path helper to vfs_* and security_* functions

[Hengqi Chen]

Add vfs_* and security_* to bpf_d_path allowlist, so that we can use
bpf_d_path helper to extract full file path from these functions' arguments.
This will help tools like BCC's filetop[1]/filelife to get full file path.

[1] https://github.com/iovisor/bcc/issues/3527

Acked-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
 kernel/trace/bpf_trace.c | 60 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 56 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index c5e0b6a64091..e7b24abcf3bf 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -849,18 +849,70 @@ BPF_CALL_3(bpf_d_path, struct path *, path, char *, buf, u32, sz)
 
 BTF_SET_START(btf_allowlist_d_path)
 #ifdef CONFIG_SECURITY
+BTF_ID(func, security_bprm_check)
+BTF_ID(func, security_bprm_committed_creds)
+BTF_ID(func, security_bprm_committing_creds)
+BTF_ID(func, security_bprm_creds_for_exec)
+BTF_ID(func, security_bprm_creds_from_file)
+BTF_ID(func, security_file_alloc)
+BTF_ID(func, security_file_fcntl)
+BTF_ID(func, security_file_free)
+BTF_ID(func, security_file_ioctl)
+BTF_ID(func, security_file_lock)
+BTF_ID(func, security_file_open)
 BTF_ID(func, security_file_permission)
+BTF_ID(func, security_file_receive)
+BTF_ID(func, security_file_set_fowner)
 BTF_ID(func, security_inode_getattr)
-BTF_ID(func, security_file_open)
+BTF_ID(func, security_sb_mount)
 #endif
 #ifdef CONFIG_SECURITY_PATH
+BTF_ID(func, security_path_chmod)
+BTF_ID(func, security_path_chown)
+BTF_ID(func, security_path_chroot)
+BTF_ID(func, security_path_link)
+BTF_ID(func, security_path_mkdir)
+BTF_ID(func, security_path_mknod)
+BTF_ID(func, security_path_notify)
+BTF_ID(func, security_path_rename)
+BTF_ID(func, security_path_rmdir)
+BTF_ID(func, security_path_symlink)
 BTF_ID(func, security_path_truncate)
+BTF_ID(func, security_path_unlink)
 #endif
-BTF_ID(func, vfs_truncate)
-BTF_ID(func, vfs_fallocate)
 BTF_ID(func, dentry_open)
-BTF_ID(func, vfs_getattr)
 BTF_ID(func, filp_close)
+BTF_ID(func, vfs_cancel_lock)
+BTF_ID(func, vfs_clone_file_range)
+BTF_ID(func, vfs_copy_file_range)
+BTF_ID(func, vfs_dedupe_file_range)
+BTF_ID(func, vfs_dedupe_file_range_one)
+BTF_ID(func, vfs_fadvise)
+BTF_ID(func, vfs_fallocate)
+BTF_ID(func, vfs_fchmod)
+BTF_ID(func, vfs_fchown)
+BTF_ID(func, vfs_fsync)
+BTF_ID(func, vfs_fsync_range)
+BTF_ID(func, vfs_getattr)
+BTF_ID(func, vfs_getattr_nosec)
+BTF_ID(func, vfs_iocb_iter_read)
+BTF_ID(func, vfs_iocb_iter_write)
+BTF_ID(func, vfs_ioctl)
+BTF_ID(func, vfs_iter_read)
+BTF_ID(func, vfs_iter_write)
+BTF_ID(func, vfs_llseek)
+BTF_ID(func, vfs_lock_file)
+BTF_ID(func, vfs_open)
+BTF_ID(func, vfs_read)
+BTF_ID(func, vfs_readv)
+BTF_ID(func, vfs_setlease)
+BTF_ID(func, vfs_setpos)
+BTF_ID(func, vfs_statfs)
+BTF_ID(func, vfs_test_lock)
+BTF_ID(func, vfs_truncate)
+BTF_ID(func, vfs_utimes)
+BTF_ID(func, vfs_write)
+BTF_ID(func, vfs_writev)
 BTF_SET_END(btf_allowlist_d_path)
 
 static bool bpf_d_path_allowed(const struct bpf_prog *prog)
-- 
2.25.1

Re: [PATCH bpf-next v4 0/2] bpf: expand bpf_d_path helper allowlist

[Andrii Nakryiko]

On Mon, Jul 26, 2021 at 7:10 AM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>
> This patch set adds more functions to bpf_d_path allowlist.
>
> Patch 1 is prep work which updates resolve_btfids to emit warnings
> on missing symbols instead of aborting kernel build process.
>
> Patch 2 expands bpf_d_path allowlist.
>
> Changes since v3: [3]
>  - Addressed Yonghong's comments. Sort allowlist and add security_bprm_*
>
> Changes since v2: [2]
>  - Andrii suggested that we should first address an issue of .BTF_ids
>    before adding more symbols to .BTF_ids. Fixed that.
>  - Yaniv proposed adding security_sb_mount and security_bprm_check.
>    Added them.
>
> Changes since v1: [1]
>  - Alexei and Yonghong suggested that bpf_d_path helper could also
>    apply to vfs_* and security_file_* kernel functions. Added them.
>
> [1] https://lore.kernel.org/bpf/20210712162424.2034006-1-hengqi.chen@gmail.com/
> [2] https://lore.kernel.org/bpf/20210719151753.399227-1-hengqi.chen@gmail.com/
> [3] https://lore.kernel.org/bpf/20210725141814.2000828-3-hengqi.chen@gmail.com/
>

I've applied the first patch to bpf-next. I'd like some more eyes on
patch #2, so I'm leaving it up for review by others for a bit longer.

> Hengqi Chen (2):
>   tools/resolve_btfids: emit warnings and patch zero id for missing
>     symbols
>   bpf: expose bpf_d_path helper to vfs_* and security_* functions
>
>  kernel/trace/bpf_trace.c        | 60 ++++++++++++++++++++++++++++++---
>  tools/bpf/resolve_btfids/main.c | 13 +++----
>  2 files changed, 63 insertions(+), 10 deletions(-)
>
> --
> 2.25.1
>