[PATCH bpf-next v2 0/7] Add check for bpf lsm return value

[KP Singh]

On Mon, Mar 25, 2024 at 10:53 AM Xu Kuohai <xukuohai at huaweicloud.com> wrote:
>
> From: Xu Kuohai <xukuohai at huawei.com>
>
> A bpf prog returning positive number attached to file_alloc_security hook
> will make kernel panic.
>
> Here is a panic log:
>
> [  441.235774] BUG: kernel NULL pointer dereference, address: 00000000000009
> [  441.236748] #PF: supervisor write access in kernel mode
> [  441.237429] #PF: error_code(0x0002) - not-present page
> [  441.238119] PGD 800000000b02f067 P4D 800000000b02f067 PUD b031067 PMD 0
> [  441.238990] Oops: 0002 [#1] PREEMPT SMP PTI
> [  441.239546] CPU: 0 PID: 347 Comm: loader Not tainted 6.8.0-rc6-gafe0cbf23373 #22
> [  441.240496] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b4
> [  441.241933] RIP: 0010:alloc_file+0x4b/0x190
> [  441.242485] Code: 8b 04 25 c0 3c 1f 00 48 8b b0 30 0c 00 00 e8 9c fe ff ff 48 3d 00 f0 ff fb
> [  441.244820] RSP: 0018:ffffc90000c67c40 EFLAGS: 00010203
> [  441.245484] RAX: ffff888006a891a0 RBX: ffffffff8223bd00 RCX: 0000000035b08000
> [  441.246391] RDX: ffff88800b95f7b0 RSI: 00000000001fc110 RDI: f089cd0b8088ffff
> [  441.247294] RBP: ffffc90000c67c58 R08: 0000000000000001 R09: 0000000000000001
> [  441.248209] R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000001
> [  441.249108] R13: ffffc90000c67c78 R14: ffffffff8223bd00 R15: fffffffffffffff4
> [  441.250007] FS:  00000000005f3300(0000) GS:ffff88803ec00000(0000) knlGS:0000000000000000
> [  441.251053] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  441.251788] CR2: 00000000000001a9 CR3: 000000000bdc4003 CR4: 0000000000170ef0
> [  441.252688] Call Trace:
> [  441.253011]  <TASK>
> [  441.253296]  ? __die+0x24/0x70
> [  441.253702]  ? page_fault_oops+0x15b/0x480
> [  441.254236]  ? fixup_exception+0x26/0x330
> [  441.254750]  ? exc_page_fault+0x6d/0x1c0
> [  441.255257]  ? asm_exc_page_fault+0x26/0x30
> [  441.255792]  ? alloc_file+0x4b/0x190
> [  441.256257]  alloc_file_pseudo+0x9f/0xf0
> [  441.256760]  __anon_inode_getfile+0x87/0x190
> [  441.257311]  ? lock_release+0x14e/0x3f0
> [  441.257808]  bpf_link_prime+0xe8/0x1d0
> [  441.258315]  bpf_tracing_prog_attach+0x311/0x570
> [  441.258916]  ? __pfx_bpf_lsm_file_alloc_security+0x10/0x10
> [  441.259605]  __sys_bpf+0x1bb7/0x2dc0
> [  441.260070]  __x64_sys_bpf+0x20/0x30
> [  441.260533]  do_syscall_64+0x72/0x140
> [  441.261004]  entry_SYSCALL_64_after_hwframe+0x6e/0x76
> [  441.261643] RIP: 0033:0x4b0349
> [  441.262045] Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 88
> [  441.264355] RSP: 002b:00007fff74daee38 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
> [  441.265293] RAX: ffffffffffffffda RBX: 00007fff74daef30 RCX: 00000000004b0349
> [  441.266187] RDX: 0000000000000040 RSI: 00007fff74daee50 RDI: 000000000000001c
> [  441.267114] RBP: 000000000000001b R08: 00000000005ef820 R09: 0000000000000000
> [  441.268018] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000004
> [  441.268907] R13: 0000000000000004 R14: 00000000005ef018 R15: 00000000004004e8
>
> 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 filesystem mistakenly
> uses this random positive number as file pointer, causing panic.
>
> To fix this issue, there are two schemes:
>
> 1. Modify the calling sites of file_alloc_security to take positive
>    return values as zero.
>
> 2. Make the bpf verifier to ensure no unpredicted value returned by
>    lsm bpf prog.
>
> Considering that hook file_alloc_security never returned positive number
> before bpf lsm was introduced, and other lsm hooks may have the same
> problem, scheme 2 is more reasonable.
>
> So this patch set adds lsm return value check in verifier to fix it.
>
> v2:
> fix bpf ci failure
>
> v1:
> https://lore.kernel.org/bpf/20240316122359.1073787-1-xukuohai@huaweicloud.com/
>
> Xu Kuohai (7):
>   bpf, lsm: Annotate lsm hook return integer with new macro LSM_RET_INT
>   bpf, lsm: Add return value range description for lsm hook
>   bpf, lsm: Add function to read lsm hook return value range
>   bpf, lsm: Check bpf lsm hook return values in verifier
>   bpf: Fix compare error in function retval_range_within
>   selftests/bpf: Avoid load failure for token_lsm.c
>   selftests/bpf: Add return value checks and corrections for failed
>     progs

This series does not apply cleanly on any of the following branches:

bpf-next
bpf
linux
linux-next
or Paul's lsm branches

There are just too many merge conflicts in the lsm_hook_defs.h file.

- KP