New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add check for bpf lsm return value #6640
Conversation
Upstream branch: 61df575 |
12747d1
to
f0f6efd
Compare
Upstream branch: 476a5e9 |
b05aa93
to
7e7a5b7
Compare
f0f6efd
to
a8f0c14
Compare
Upstream branch: a849750 |
7e7a5b7
to
4eba9cc
Compare
a8f0c14
to
283e9c4
Compare
Upstream branch: 770546a |
4eba9cc
to
0f387ab
Compare
283e9c4
to
a4e5017
Compare
Upstream branch: 14bb1e8 |
0f387ab
to
283dc68
Compare
a4e5017
to
388b228
Compare
Upstream branch: e0754c1 |
283dc68
to
f1302f3
Compare
388b228
to
367dff7
Compare
Add macro LSM_RET_INT to annotate lsm hook return integer type and the default return value. The follow-up patch will use this macro to add return value range descriptions. Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
Add return value descriptions for lsm hook. Two integer ranges are added: 1. ERRNO: Integer between -MAX_ERRNO and 0, including -MAX_ERRNO and 0. 2. ANY: Any integer Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
Add function to read lsm hook return value range. The following patch will use this information to check lsm hook return values in bpf verifier. Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
A bpf prog returning positive number attached to file_alloc_security hook will make kernel panic. The reason is that the positive number returned by bpf prog is not a valid errno, and could not be filtered out with IS_ERR which is used by the file system to check errors. As a result, the file system uses this positive number as file pointer, causing panic. Considering that hook file_alloc_security never returned positive number before bpf lsm was introduced, and other bpf lsm hooks may have the same problem, this patch adds lsm return value check in bpf verifier to ensure no unpredicted values will be returned by lsm bpf prog. Fixes: 520b7aa ("bpf: lsm: Initialize the BPF LSM hooks") Reported-by: Xin Liu <liuxin350@huawei.com> Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
After checking lsm hook return range in verifier, the test case "test_progs -t test_lsm" failed, and the failure log says: libbpf: prog 'test_int_hook': BPF program load failed: Invalid argument libbpf: prog 'test_int_hook': -- BEGIN PROG LOAD LOG -- 0: R1=ctx() R10=fp0 ; int BPF_PROG(test_int_hook, struct vm_area_struct *vma, @ lsm.c:89 0: (79) r0 = *(u64 *)(r1 +24) ; R0_w=scalar(smin=smin32=-4095,smax=smax32=0) R1=ctx() [...] 24: (b4) w0 = -1 ; R0_w=0xffffffff ; int BPF_PROG(test_int_hook, struct vm_area_struct *vma, @ lsm.c:89 25: (95) exit At program exit the register R0 has smin=4294967295 smax=4294967295 should have been in [-4095, 0] It can be seen that instruction "w0 = -1" zero extended -1 to 64-bit register r0, setting both smin and smax values of r0 to 4294967295. This resulted in a false reject when r0 was checked with range [-4095, 0]. Given bpf_retval_range is a 32-bit range, this patch fixes it by changing the compare between r0 and return range from 64-bit operation to 32-bit operation. Fixes: 8fa4ecd ("bpf: enforce exact retval range on subprog/callback exit") Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
The compiler optimized the two bpf progs in token_lsm.c to make return value from the bool variable in the "return -1" path, causing an unexpected rejection: 0: R1=ctx() R10=fp0 ; int BPF_PROG(bpf_token_capable, struct bpf_token *token, int cap) @ bpf_lsm.c:17 0: (b7) r6 = 0 ; R6_w=0 ; if (my_pid == 0 || my_pid != (bpf_get_current_pid_tgid() >> 32)) @ bpf_lsm.c:19 1: (18) r1 = 0xffffc9000102a000 ; R1_w=map_value(map=bpf_lsm.bss,ks=4,vs=5) 3: (61) r7 = *(u32 *)(r1 +0) ; R1_w=map_value(map=bpf_lsm.bss,ks=4,vs=5) R7_w=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff)) 4: (15) if r7 == 0x0 goto pc+11 ; R7_w=scalar(smin=umin=umin32=1,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff)) 5: (67) r7 <<= 32 ; R7_w=scalar(smax=0x7fffffff00000000,umax=0xffffffff00000000,smin32=0,smax32=umax32=0,var_off=(0x0; 0xffffffff00000000)) 6: (c7) r7 s>>= 32 ; R7_w=scalar(smin=0xffffffff80000000,smax=0x7fffffff) 7: (85) call bpf_get_current_pid_tgid#14 ; R0=scalar() 8: (77) r0 >>= 32 ; R0_w=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff)) 9: (5d) if r0 != r7 goto pc+6 ; R0_w=scalar(smin=smin32=0,smax=umax=umax32=0x7fffffff,var_off=(0x0; 0x7fffffff)) R7=scalar(smin=smin32=0,smax=umax=umax32=0x7fffffff,var_off=(0x0; 0x7fffffff)) ; if (reject_capable) @ bpf_lsm.c:21 10: (18) r1 = 0xffffc9000102a004 ; R1_w=map_value(map=bpf_lsm.bss,ks=4,vs=5,off=4) 12: (71) r6 = *(u8 *)(r1 +0) ; R1_w=map_value(map=bpf_lsm.bss,ks=4,vs=5,off=4) R6_w=scalar(smin=smin32=0,smax=umax=smax32=umax32=255,var_off=(0x0; 0xff)) ; @ bpf_lsm.c:0 13: (87) r6 = -r6 ; R6_w=scalar() 14: (67) r6 <<= 56 ; R6_w=scalar(smax=0x7f00000000000000,umax=0xff00000000000000,smin32=0,smax32=umax32=0,var_off=(0x0; 0xff00000000000000)) 15: (c7) r6 s>>= 56 ; R6_w=scalar(smin=smin32=-128,smax=smax32=127) ; int BPF_PROG(bpf_token_capable, struct bpf_token *token, int cap) @ bpf_lsm.c:17 16: (bf) r0 = r6 ; R0_w=scalar(id=1,smin=smin32=-128,smax=smax32=127) R6_w=scalar(id=1,smin=smin32=-128,smax=smax32=127) 17: (95) exit At program exit the register R0 has smin=-128 smax=127 should have been in [-4095, 0] To avoid this failure, change the variable type from bool to int. Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
e4e212b
to
fa3d31e
Compare
fa3d31e
to
5fc06c3
Compare
At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=837810 expired. Closing PR. |
Pull request for series with
subject: Add check for bpf lsm return value
version: 2
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=837810