Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

bpf: Do not mark certain LSM hook arguments as trusted

Martin mentioned that the verifier cannot assume arguments from
LSM hook sk_alloc_security being trusted since after the hook
is called, the sk ref_count is set to 1. This will overwrite
the ref_count changed by the bpf program and may cause ref_count
underflow later on.

I then further checked some other hooks. For example,
for bpf_lsm_file_alloc() hook in fs/file_table.c,

f->f_cred = get_cred(cred);
error = security_file_alloc(f);
if (unlikely(error)) {
file_free_rcu(&f->f_rcuhead);
return ERR_PTR(error);
}

atomic_long_set(&f->f_count, 1);

The input parameter 'f' to security_file_alloc() cannot be trusted
as well.

Specifically, I investiaged bpf_map/bpf_prog/file/sk/task alloc/free
lsm hooks. Except bpf_map_alloc and task_alloc, arguments for all other
hooks should not be considered as trusted. This may not be a complete
list, but it covers common usage for sk and task.

Fixes: 3f00c5239344 ("bpf: Allow trusted pointers to be passed to KF_TRUSTED_ARGS kfuncs")
Signed-off-by: Yonghong Song <yhs@fb.com>
Link: https://lore.kernel.org/r/20221203204954.2043348-1-yhs@fb.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

authored by

Yonghong Song and committed by
Alexei Starovoitov
c0c852dd 1910676c

+36
+6
include/linux/bpf_lsm.h
··· 28 28 const struct bpf_prog *prog); 29 29 30 30 bool bpf_lsm_is_sleepable_hook(u32 btf_id); 31 + bool bpf_lsm_is_trusted(const struct bpf_prog *prog); 31 32 32 33 static inline struct bpf_storage_blob *bpf_inode( 33 34 const struct inode *inode) ··· 48 47 #else /* !CONFIG_BPF_LSM */ 49 48 50 49 static inline bool bpf_lsm_is_sleepable_hook(u32 btf_id) 50 + { 51 + return false; 52 + } 53 + 54 + static inline bool bpf_lsm_is_trusted(const struct bpf_prog *prog) 51 55 { 52 56 return false; 53 57 }
+16
kernel/bpf/bpf_lsm.c
··· 345 345 BTF_ID(func, bpf_lsm_userns_create) 346 346 BTF_SET_END(sleepable_lsm_hooks) 347 347 348 + BTF_SET_START(untrusted_lsm_hooks) 349 + BTF_ID(func, bpf_lsm_bpf_map_free_security) 350 + BTF_ID(func, bpf_lsm_bpf_prog_alloc_security) 351 + BTF_ID(func, bpf_lsm_bpf_prog_free_security) 352 + BTF_ID(func, bpf_lsm_file_alloc_security) 353 + BTF_ID(func, bpf_lsm_file_free_security) 354 + BTF_ID(func, bpf_lsm_sk_alloc_security) 355 + BTF_ID(func, bpf_lsm_sk_free_security) 356 + BTF_ID(func, bpf_lsm_task_free) 357 + BTF_SET_END(untrusted_lsm_hooks) 358 + 348 359 bool bpf_lsm_is_sleepable_hook(u32 btf_id) 349 360 { 350 361 return btf_id_set_contains(&sleepable_lsm_hooks, btf_id); 362 + } 363 + 364 + bool bpf_lsm_is_trusted(const struct bpf_prog *prog) 365 + { 366 + return !btf_id_set_contains(&untrusted_lsm_hooks, prog->aux->attach_btf_id); 351 367 } 352 368 353 369 const struct bpf_prog_ops lsm_prog_ops = {
+2
kernel/bpf/btf.c
··· 19 19 #include <linux/bpf_verifier.h> 20 20 #include <linux/btf.h> 21 21 #include <linux/btf_ids.h> 22 + #include <linux/bpf_lsm.h> 22 23 #include <linux/skmsg.h> 23 24 #include <linux/perf_event.h> 24 25 #include <linux/bsearch.h> ··· 5830 5829 case BPF_PROG_TYPE_TRACING: 5831 5830 return atype == BPF_TRACE_RAW_TP || atype == BPF_TRACE_ITER; 5832 5831 case BPF_PROG_TYPE_LSM: 5832 + return bpf_lsm_is_trusted(prog); 5833 5833 case BPF_PROG_TYPE_STRUCT_OPS: 5834 5834 return true; 5835 5835 default:
+1
tools/testing/selftests/bpf/prog_tests/task_kfunc.c
··· 103 103 {"task_kfunc_release_null", "arg#0 is ptr_or_null_ expected ptr_ or socket"}, 104 104 {"task_kfunc_release_unacquired", "release kernel function bpf_task_release expects"}, 105 105 {"task_kfunc_from_pid_no_null_check", "arg#0 is ptr_or_null_ expected ptr_ or socket"}, 106 + {"task_kfunc_from_lsm_task_free", "reg type unsupported for arg#0 function"}, 106 107 }; 107 108 108 109 static void verify_fail(const char *prog_name, const char *expected_err_msg)
+11
tools/testing/selftests/bpf/progs/task_kfunc_failure.c
··· 271 271 272 272 return 0; 273 273 } 274 + 275 + SEC("lsm/task_free") 276 + int BPF_PROG(task_kfunc_from_lsm_task_free, struct task_struct *task) 277 + { 278 + struct task_struct *acquired; 279 + 280 + /* the argument of lsm task_free hook is untrusted. */ 281 + acquired = bpf_task_acquire(task); 282 + bpf_task_release(acquired); 283 + return 0; 284 + }