* BPF Kernel OOPS - NULL pointer dereference
@ 2020-12-20 10:16 Gilad Reti
2021-01-06 10:14 ` Gilad Reti
0 siblings, 1 reply; 5+ messages in thread
From: Gilad Reti @ 2020-12-20 10:16 UTC (permalink / raw)
To: bpf
Hello there,
During experimenting with the new inode local storage I have stumbled
across an invalid pointer dereference that passed through the
verifier.
After investigating, I think that it happens in the
bpf_inode_storage_get helper, and in particular in the following line:
https://elixir.bootlin.com/linux/v5.10.1/source/include/linux/bpf_lsm.h#L32
I have a single bpf lsm probe in the security_inode_rename probe, and
I have called bpf_inode_storage_get on the inode of the "new_dentry"
argument. This inode may be null in cases where renameat(2) is called
to move a file from one path to a new path which didn't exist before.
As can be seen, inode is dereferenced without first checking that it
is not NULL.
I don't know what should be the correct behavior, but I believe that
either the helper should check the validity of passed pointers, or the
verifier should treat fields of BTF pointers (PTR_TO_BTF_ID) as
PTR_TO_BTF_ID_OR_NULL.
I am attaching a minimal program example, along with the kernel demsg
output. To reproduce, load the probe and run "mv file1 file2" where
file2 does not exist.
Thanks,
Gilad Reti
inode_oops.c:
// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
/* Copyright (c) 2020 Facebook */
#include <argp.h>
#include <signal.h>
#include <stdio.h>
#include <time.h>
#include <unistd.h>
#include <sys/resource.h>
#include <bpf/libbpf.h>
#include "inode_oops.skel.h"
static volatile bool exiting = false;
static void sig_handler(int sig)
{
exiting = true;
}
int main(int argc, char **argv)
{
struct inode_oops_bpf *skel;
int err;
/* Cleaner handling of Ctrl-C */
signal(SIGINT, sig_handler);
signal(SIGTERM, sig_handler);
/* Load and verify BPF application */
skel = inode_oops_bpf__open_and_load();
if (!skel)
{
fprintf(stderr, "Failed to open and load BPF skeleton\n");
goto cleanup;
}
/* Attach tracepoints */
err = inode_oops_bpf__attach(skel);
if (err)
{
fprintf(stderr, "Failed to attach BPF skeleton\n");
goto cleanup;
}
while (!exiting)
{
}
cleanup:
/* Clean up */
inode_oops_bpf__destroy(skel);
return err < 0 ? -err : 0;
}
inode_oops.bpf.c
#include "vmlinux.h"
#include <bpf/bpf_core_read.h>
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
struct dummy_storage
{
__u32 value;
};
struct
{
__uint(type, BPF_MAP_TYPE_INODE_STORAGE);
__uint(map_flags, BPF_F_NO_PREALLOC);
__type(key, int);
__type(value, struct dummy_storage);
} inode_storage_map SEC(".maps");
SEC("lsm/inode_rename")
int BPF_PROG(inode_rename, struct inode *old_dir, struct dentry *old_dentry,
struct inode *new_dir, struct dentry *new_dentry,
unsigned int flags)
{
bpf_inode_storage_get(&inode_storage_map, new_dentry->d_inode, 0,
BPF_LOCAL_STORAGE_GET_F_CREATE);
return 0;
}
Dmesg -T:
[Thu Dec 17 11:35:37 2020] BUG: kernel NULL pointer dereference,
address: 0000000000000038
[Thu Dec 17 11:35:37 2020] #PF: supervisor read access in kernel mode
[Thu Dec 17 11:35:37 2020] #PF: error_code(0x0000) - not-present page
[Thu Dec 17 11:35:37 2020] PGD 0 P4D 0
[Thu Dec 17 11:35:37 2020] Oops: 0000 [#1] SMP PTI
[Thu Dec 17 11:35:37 2020] CPU: 0 PID: 4437 Comm: bash Not tainted 5.10.0 #17
[Thu Dec 17 11:35:37 2020] Hardware name: VMware, Inc. VMware Virtual
Platform/440BX Desktop Reference Platform, BIOS 6.00 02/27/2020
[Thu Dec 17 11:35:37 2020] RIP: 0010:bpf_inode_storage_get+0x1f/0xa0
[Thu Dec 17 11:35:37 2020] Code: 65 f8 c9 c3 0f 1f 80 00 00 00 00 0f
1f 44 00 00 48 f7 c1 fe ff ff ff 74 03 31 c0 c3 55 48 89 e5 41 56 41
55 41 54 49 89 f4 53 <48> 8b 46 38 48 85 c0 74 41 49 89 d6 48 63 15 e6
d7 28 01 48 01 d0
[Thu Dec 17 11:35:37 2020] RSP: 0018:ffff9866433335f8 EFLAGS: 00010246
[Thu Dec 17 11:35:37 2020] RAX: 0000000000000000 RBX: 0000000000000001
RCX: 0000000000000000
[Thu Dec 17 11:35:37 2020] RDX: 0000000000000000 RSI: 0000000000000000
RDI: ffff8959e2db9e00
[Thu Dec 17 11:35:37 2020] RBP: ffff986643333618 R08: 0000000000000007
R09: 0000000000000003
[Thu Dec 17 11:35:37 2020] R10: ffff9866433336c3 R11: 0000000000000000
R12: 0000000000000000
[Thu Dec 17 11:35:37 2020] R13: ffff8959e2373428 R14: ffff8959e24b3d40
R15: 0000000000000008
[Thu Dec 17 11:35:37 2020] FS: 00007fa878948740(0000)
GS:ffff895cede00000(0000) knlGS:0000000000000000
[Thu Dec 17 11:35:37 2020] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[Thu Dec 17 11:35:37 2020] CR2: 0000000000000038 CR3: 0000000121f34001
CR4: 00000000003706f0
[Thu Dec 17 11:35:37 2020] Call Trace:
[Thu Dec 17 11:35:37 2020] bpf_prog_f2cb36e361a3c858_inode_rename+0x88c/0xf38
[Thu Dec 17 11:35:37 2020] ? security_capable+0x3d/0x60
[Thu Dec 17 11:35:37 2020] ? from_kgid+0x12/0x20
[Thu Dec 17 11:35:37 2020] ? capable_wrt_inode_uidgid+0x6f/0x80
[Thu Dec 17 11:35:37 2020] bpf_trampoline_22333+0x30/0x1000
[Thu Dec 17 11:35:37 2020] bpf_lsm_inode_rename+0x5/0x10
[Thu Dec 17 11:35:37 2020] ? security_inode_rename+0x88/0xb0
[Thu Dec 17 11:35:37 2020] vfs_rename+0x11b/0xb60
[Thu Dec 17 11:35:37 2020] ovl_copy_up_one+0x461/0xfc0 [overlay]
[Thu Dec 17 11:35:37 2020] ? remove_wait_queue+0x47/0x50
[Thu Dec 17 11:35:37 2020] ? fput+0x13/0x20
[Thu Dec 17 11:35:37 2020] ? poll_freewait+0x4a/0xa0
[Thu Dec 17 11:35:37 2020] ovl_copy_up_flags+0xb0/0xf0 [overlay]
[Thu Dec 17 11:35:37 2020] ovl_copy_up+0x10/0x20 [overlay]
[Thu Dec 17 11:35:37 2020] ovl_create_or_link+0x40/0x8c0 [overlay]
[Thu Dec 17 11:35:37 2020] ? security_inode_alloc+0x4b/0x90
[Thu Dec 17 11:35:37 2020] ? inode_init_always+0x137/0x210
[Thu Dec 17 11:35:37 2020] ? alloc_inode+0x35/0xc0
[Thu Dec 17 11:35:37 2020] ? new_inode+0x74/0xc0
[Thu Dec 17 11:35:37 2020] ovl_create_object+0xe1/0x110 [overlay]
[Thu Dec 17 11:35:37 2020] ovl_create+0x23/0x30 [overlay]
[Thu Dec 17 11:35:37 2020] path_openat+0xdec/0x1130
[Thu Dec 17 11:35:37 2020] ? __check_object_size+0x13f/0x150
[Thu Dec 17 11:35:37 2020] do_filp_open+0x8c/0x130
[Thu Dec 17 11:35:37 2020] ? __check_object_size+0x13f/0x150
[Thu Dec 17 11:35:37 2020] do_sys_openat2+0x9b/0x150
[Thu Dec 17 11:35:37 2020] __x64_sys_openat+0x56/0x90
[Thu Dec 17 11:35:37 2020] do_syscall_64+0x38/0x90
[Thu Dec 17 11:35:37 2020] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[Thu Dec 17 11:35:37 2020] RIP: 0033:0x7fa878018d5e
[Thu Dec 17 11:35:37 2020] Code: 25 00 00 41 00 3d 00 00 41 00 74 48
48 8d 05 91 0c 2e 00 8b 00 85 c0 75 69 89 f2 b8 01 01 00 00 48 89 fe
bf 9c ff ff ff 0f 05 <48> 3d 00 f0 ff ff 0f 87 a6 00 00 00 48 8b 4c 24
28 64 48 33 0c 25
[Thu Dec 17 11:35:37 2020] RSP: 002b:00007ffead706e80 EFLAGS: 00000246
ORIG_RAX: 0000000000000101
[Thu Dec 17 11:35:37 2020] RAX: ffffffffffffffda RBX: 0000557d910892e0
RCX: 00007fa878018d5e
[Thu Dec 17 11:35:37 2020] RDX: 0000000000000241 RSI: 0000557d910892e0
RDI: 00000000ffffff9c
[Thu Dec 17 11:35:37 2020] RBP: 0000000000000001 R08: 00007fa8782f68b0
R09: 00007fa878948740
[Thu Dec 17 11:35:37 2020] R10: 0000000000000180 R11: 0000000000000246
R12: 0000000000000000
[Thu Dec 17 11:35:37 2020] R13: 00007ffead707150 R14: 0000557d8f8cfcc0
R15: 0000000000000000
[Thu Dec 17 11:35:37 2020] Modules linked in: xt_nat veth vxlan
ip6_udp_tunnel udp_tunnel xt_policy xt_mark xt_u32 xt_tcpudp
xt_conntrack xt_MASQUERADE nf_conntrack_netlink xfrm_user xfrm_algo
nft_counter xt_addrtype nft_compat nft_chain_nat nf_nat nf_conntrack
nf_defrag_ipv6 nf_defrag_ipv4 nf_tables libcrc32c nfnetlink
br_netfilter bridge stp llc intel_rapl_msr intel_rapl_common
crct10dif_pclmul ghash_clmulni_intel snd_ens1371 overlay aesni_intel
crypto_simd snd_ac97_codec vmw_balloon cryptd vsock_loopback gameport
vmw_vsock_virtio_transport_common ac97_bus glue_helper snd_pcm
vmw_vsock_vmci_transport vsock rapl snd_seq_midi snd_seq_midi_event
snd_rawmidi snd_seq joydev input_leds serio_raw snd_seq_device
snd_timer binfmt_misc snd soundcore vmw_vmci mac_hid sch_fq_codel
vmwgfx ttm drm_kms_helper cec rc_core drm fb_sys_fops syscopyarea
sysfillrect sysimgblt parport_pc ppdev lp parport ip_tables x_tables
autofs4 hid_generic usbhid hid psmouse mptspi crc32_pclmul mptscsih
mptbase e1000 i2c_piix4 pata_acpi
[Thu Dec 17 11:35:37 2020] ahci libahci scsi_transport_spi
[Thu Dec 17 11:35:37 2020] CR2: 0000000000000038
[Thu Dec 17 11:35:37 2020] ---[ end trace fe83c4e3d6415215 ]---
[Thu Dec 17 11:35:37 2020] RIP: 0010:bpf_inode_storage_get+0x1f/0xa0
[Thu Dec 17 11:35:37 2020] Code: 65 f8 c9 c3 0f 1f 80 00 00 00 00 0f
1f 44 00 00 48 f7 c1 fe ff ff ff 74 03 31 c0 c3 55 48 89 e5 41 56 41
55 41 54 49 89 f4 53 <48> 8b 46 38 48 85 c0 74 41 49 89 d6 48 63 15 e6
d7 28 01 48 01 d0
[Thu Dec 17 11:35:37 2020] RSP: 0018:ffff9866433335f8 EFLAGS: 00010246
[Thu Dec 17 11:35:37 2020] RAX: 0000000000000000 RBX: 0000000000000001
RCX: 0000000000000000
[Thu Dec 17 11:35:37 2020] RDX: 0000000000000000 RSI: 0000000000000000
RDI: ffff8959e2db9e00
[Thu Dec 17 11:35:37 2020] RBP: ffff986643333618 R08: 0000000000000007
R09: 0000000000000003
[Thu Dec 17 11:35:37 2020] R10: ffff9866433336c3 R11: 0000000000000000
R12: 0000000000000000
[Thu Dec 17 11:35:37 2020] R13: ffff8959e2373428 R14: ffff8959e24b3d40
R15: 0000000000000008
[Thu Dec 17 11:35:37 2020] FS: 00007fa878948740(0000)
GS:ffff895cede00000(0000) knlGS:0000000000000000
[Thu Dec 17 11:35:37 2020] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[Thu Dec 17 11:35:37 2020] CR2: 0000000000000038 CR3: 0000000121f34001
CR4: 00000000003706f0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: BPF Kernel OOPS - NULL pointer dereference
2020-12-20 10:16 BPF Kernel OOPS - NULL pointer dereference Gilad Reti
@ 2021-01-06 10:14 ` Gilad Reti
2021-01-06 17:26 ` Alexei Starovoitov
0 siblings, 1 reply; 5+ messages in thread
From: Gilad Reti @ 2021-01-06 10:14 UTC (permalink / raw)
To: bpf
Hello all,
Hope you had a great new year vacation.
Sorry for bumping up the thread, but I believe that it is an important one.
Let me know if I am mistaken.
Thanks.
On Sun, Dec 20, 2020 at 12:16 PM Gilad Reti <gilad.reti@gmail.com> wrote:
>
> Hello there,
>
> During experimenting with the new inode local storage I have stumbled
> across an invalid pointer dereference that passed through the
> verifier.
>
> After investigating, I think that it happens in the
> bpf_inode_storage_get helper, and in particular in the following line:
> https://elixir.bootlin.com/linux/v5.10.1/source/include/linux/bpf_lsm.h#L32
>
> I have a single bpf lsm probe in the security_inode_rename probe, and
> I have called bpf_inode_storage_get on the inode of the "new_dentry"
> argument. This inode may be null in cases where renameat(2) is called
> to move a file from one path to a new path which didn't exist before.
>
> As can be seen, inode is dereferenced without first checking that it
> is not NULL.
> I don't know what should be the correct behavior, but I believe that
> either the helper should check the validity of passed pointers, or the
> verifier should treat fields of BTF pointers (PTR_TO_BTF_ID) as
> PTR_TO_BTF_ID_OR_NULL.
>
> I am attaching a minimal program example, along with the kernel demsg
> output. To reproduce, load the probe and run "mv file1 file2" where
> file2 does not exist.
>
> Thanks,
> Gilad Reti
>
> inode_oops.c:
>
> // SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> /* Copyright (c) 2020 Facebook */
> #include <argp.h>
> #include <signal.h>
> #include <stdio.h>
> #include <time.h>
> #include <unistd.h>
> #include <sys/resource.h>
> #include <bpf/libbpf.h>
>
> #include "inode_oops.skel.h"
>
> static volatile bool exiting = false;
>
> static void sig_handler(int sig)
> {
> exiting = true;
> }
>
> int main(int argc, char **argv)
> {
> struct inode_oops_bpf *skel;
>
> int err;
>
> /* Cleaner handling of Ctrl-C */
> signal(SIGINT, sig_handler);
> signal(SIGTERM, sig_handler);
>
> /* Load and verify BPF application */
> skel = inode_oops_bpf__open_and_load();
> if (!skel)
> {
> fprintf(stderr, "Failed to open and load BPF skeleton\n");
> goto cleanup;
> }
>
> /* Attach tracepoints */
> err = inode_oops_bpf__attach(skel);
> if (err)
> {
> fprintf(stderr, "Failed to attach BPF skeleton\n");
> goto cleanup;
> }
>
> while (!exiting)
> {
> }
> cleanup:
> /* Clean up */
> inode_oops_bpf__destroy(skel);
>
> return err < 0 ? -err : 0;
> }
>
> inode_oops.bpf.c
>
> #include "vmlinux.h"
> #include <bpf/bpf_core_read.h>
> #include <bpf/bpf_helpers.h>
> #include <bpf/bpf_tracing.h>
>
> struct dummy_storage
> {
> __u32 value;
> };
>
> struct
> {
> __uint(type, BPF_MAP_TYPE_INODE_STORAGE);
> __uint(map_flags, BPF_F_NO_PREALLOC);
> __type(key, int);
> __type(value, struct dummy_storage);
> } inode_storage_map SEC(".maps");
>
> SEC("lsm/inode_rename")
> int BPF_PROG(inode_rename, struct inode *old_dir, struct dentry *old_dentry,
> struct inode *new_dir, struct dentry *new_dentry,
> unsigned int flags)
> {
>
> bpf_inode_storage_get(&inode_storage_map, new_dentry->d_inode, 0,
> BPF_LOCAL_STORAGE_GET_F_CREATE);
>
> return 0;
> }
>
> Dmesg -T:
>
> [Thu Dec 17 11:35:37 2020] BUG: kernel NULL pointer dereference,
> address: 0000000000000038
> [Thu Dec 17 11:35:37 2020] #PF: supervisor read access in kernel mode
> [Thu Dec 17 11:35:37 2020] #PF: error_code(0x0000) - not-present page
> [Thu Dec 17 11:35:37 2020] PGD 0 P4D 0
> [Thu Dec 17 11:35:37 2020] Oops: 0000 [#1] SMP PTI
> [Thu Dec 17 11:35:37 2020] CPU: 0 PID: 4437 Comm: bash Not tainted 5.10.0 #17
> [Thu Dec 17 11:35:37 2020] Hardware name: VMware, Inc. VMware Virtual
> Platform/440BX Desktop Reference Platform, BIOS 6.00 02/27/2020
> [Thu Dec 17 11:35:37 2020] RIP: 0010:bpf_inode_storage_get+0x1f/0xa0
> [Thu Dec 17 11:35:37 2020] Code: 65 f8 c9 c3 0f 1f 80 00 00 00 00 0f
> 1f 44 00 00 48 f7 c1 fe ff ff ff 74 03 31 c0 c3 55 48 89 e5 41 56 41
> 55 41 54 49 89 f4 53 <48> 8b 46 38 48 85 c0 74 41 49 89 d6 48 63 15 e6
> d7 28 01 48 01 d0
> [Thu Dec 17 11:35:37 2020] RSP: 0018:ffff9866433335f8 EFLAGS: 00010246
> [Thu Dec 17 11:35:37 2020] RAX: 0000000000000000 RBX: 0000000000000001
> RCX: 0000000000000000
> [Thu Dec 17 11:35:37 2020] RDX: 0000000000000000 RSI: 0000000000000000
> RDI: ffff8959e2db9e00
> [Thu Dec 17 11:35:37 2020] RBP: ffff986643333618 R08: 0000000000000007
> R09: 0000000000000003
> [Thu Dec 17 11:35:37 2020] R10: ffff9866433336c3 R11: 0000000000000000
> R12: 0000000000000000
> [Thu Dec 17 11:35:37 2020] R13: ffff8959e2373428 R14: ffff8959e24b3d40
> R15: 0000000000000008
> [Thu Dec 17 11:35:37 2020] FS: 00007fa878948740(0000)
> GS:ffff895cede00000(0000) knlGS:0000000000000000
> [Thu Dec 17 11:35:37 2020] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [Thu Dec 17 11:35:37 2020] CR2: 0000000000000038 CR3: 0000000121f34001
> CR4: 00000000003706f0
> [Thu Dec 17 11:35:37 2020] Call Trace:
> [Thu Dec 17 11:35:37 2020] bpf_prog_f2cb36e361a3c858_inode_rename+0x88c/0xf38
> [Thu Dec 17 11:35:37 2020] ? security_capable+0x3d/0x60
> [Thu Dec 17 11:35:37 2020] ? from_kgid+0x12/0x20
> [Thu Dec 17 11:35:37 2020] ? capable_wrt_inode_uidgid+0x6f/0x80
> [Thu Dec 17 11:35:37 2020] bpf_trampoline_22333+0x30/0x1000
> [Thu Dec 17 11:35:37 2020] bpf_lsm_inode_rename+0x5/0x10
> [Thu Dec 17 11:35:37 2020] ? security_inode_rename+0x88/0xb0
> [Thu Dec 17 11:35:37 2020] vfs_rename+0x11b/0xb60
> [Thu Dec 17 11:35:37 2020] ovl_copy_up_one+0x461/0xfc0 [overlay]
> [Thu Dec 17 11:35:37 2020] ? remove_wait_queue+0x47/0x50
> [Thu Dec 17 11:35:37 2020] ? fput+0x13/0x20
> [Thu Dec 17 11:35:37 2020] ? poll_freewait+0x4a/0xa0
> [Thu Dec 17 11:35:37 2020] ovl_copy_up_flags+0xb0/0xf0 [overlay]
> [Thu Dec 17 11:35:37 2020] ovl_copy_up+0x10/0x20 [overlay]
> [Thu Dec 17 11:35:37 2020] ovl_create_or_link+0x40/0x8c0 [overlay]
> [Thu Dec 17 11:35:37 2020] ? security_inode_alloc+0x4b/0x90
> [Thu Dec 17 11:35:37 2020] ? inode_init_always+0x137/0x210
> [Thu Dec 17 11:35:37 2020] ? alloc_inode+0x35/0xc0
> [Thu Dec 17 11:35:37 2020] ? new_inode+0x74/0xc0
> [Thu Dec 17 11:35:37 2020] ovl_create_object+0xe1/0x110 [overlay]
> [Thu Dec 17 11:35:37 2020] ovl_create+0x23/0x30 [overlay]
> [Thu Dec 17 11:35:37 2020] path_openat+0xdec/0x1130
> [Thu Dec 17 11:35:37 2020] ? __check_object_size+0x13f/0x150
> [Thu Dec 17 11:35:37 2020] do_filp_open+0x8c/0x130
> [Thu Dec 17 11:35:37 2020] ? __check_object_size+0x13f/0x150
> [Thu Dec 17 11:35:37 2020] do_sys_openat2+0x9b/0x150
> [Thu Dec 17 11:35:37 2020] __x64_sys_openat+0x56/0x90
> [Thu Dec 17 11:35:37 2020] do_syscall_64+0x38/0x90
> [Thu Dec 17 11:35:37 2020] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [Thu Dec 17 11:35:37 2020] RIP: 0033:0x7fa878018d5e
> [Thu Dec 17 11:35:37 2020] Code: 25 00 00 41 00 3d 00 00 41 00 74 48
> 48 8d 05 91 0c 2e 00 8b 00 85 c0 75 69 89 f2 b8 01 01 00 00 48 89 fe
> bf 9c ff ff ff 0f 05 <48> 3d 00 f0 ff ff 0f 87 a6 00 00 00 48 8b 4c 24
> 28 64 48 33 0c 25
> [Thu Dec 17 11:35:37 2020] RSP: 002b:00007ffead706e80 EFLAGS: 00000246
> ORIG_RAX: 0000000000000101
> [Thu Dec 17 11:35:37 2020] RAX: ffffffffffffffda RBX: 0000557d910892e0
> RCX: 00007fa878018d5e
> [Thu Dec 17 11:35:37 2020] RDX: 0000000000000241 RSI: 0000557d910892e0
> RDI: 00000000ffffff9c
> [Thu Dec 17 11:35:37 2020] RBP: 0000000000000001 R08: 00007fa8782f68b0
> R09: 00007fa878948740
> [Thu Dec 17 11:35:37 2020] R10: 0000000000000180 R11: 0000000000000246
> R12: 0000000000000000
> [Thu Dec 17 11:35:37 2020] R13: 00007ffead707150 R14: 0000557d8f8cfcc0
> R15: 0000000000000000
> [Thu Dec 17 11:35:37 2020] Modules linked in: xt_nat veth vxlan
> ip6_udp_tunnel udp_tunnel xt_policy xt_mark xt_u32 xt_tcpudp
> xt_conntrack xt_MASQUERADE nf_conntrack_netlink xfrm_user xfrm_algo
> nft_counter xt_addrtype nft_compat nft_chain_nat nf_nat nf_conntrack
> nf_defrag_ipv6 nf_defrag_ipv4 nf_tables libcrc32c nfnetlink
> br_netfilter bridge stp llc intel_rapl_msr intel_rapl_common
> crct10dif_pclmul ghash_clmulni_intel snd_ens1371 overlay aesni_intel
> crypto_simd snd_ac97_codec vmw_balloon cryptd vsock_loopback gameport
> vmw_vsock_virtio_transport_common ac97_bus glue_helper snd_pcm
> vmw_vsock_vmci_transport vsock rapl snd_seq_midi snd_seq_midi_event
> snd_rawmidi snd_seq joydev input_leds serio_raw snd_seq_device
> snd_timer binfmt_misc snd soundcore vmw_vmci mac_hid sch_fq_codel
> vmwgfx ttm drm_kms_helper cec rc_core drm fb_sys_fops syscopyarea
> sysfillrect sysimgblt parport_pc ppdev lp parport ip_tables x_tables
> autofs4 hid_generic usbhid hid psmouse mptspi crc32_pclmul mptscsih
> mptbase e1000 i2c_piix4 pata_acpi
> [Thu Dec 17 11:35:37 2020] ahci libahci scsi_transport_spi
> [Thu Dec 17 11:35:37 2020] CR2: 0000000000000038
> [Thu Dec 17 11:35:37 2020] ---[ end trace fe83c4e3d6415215 ]---
> [Thu Dec 17 11:35:37 2020] RIP: 0010:bpf_inode_storage_get+0x1f/0xa0
> [Thu Dec 17 11:35:37 2020] Code: 65 f8 c9 c3 0f 1f 80 00 00 00 00 0f
> 1f 44 00 00 48 f7 c1 fe ff ff ff 74 03 31 c0 c3 55 48 89 e5 41 56 41
> 55 41 54 49 89 f4 53 <48> 8b 46 38 48 85 c0 74 41 49 89 d6 48 63 15 e6
> d7 28 01 48 01 d0
> [Thu Dec 17 11:35:37 2020] RSP: 0018:ffff9866433335f8 EFLAGS: 00010246
> [Thu Dec 17 11:35:37 2020] RAX: 0000000000000000 RBX: 0000000000000001
> RCX: 0000000000000000
> [Thu Dec 17 11:35:37 2020] RDX: 0000000000000000 RSI: 0000000000000000
> RDI: ffff8959e2db9e00
> [Thu Dec 17 11:35:37 2020] RBP: ffff986643333618 R08: 0000000000000007
> R09: 0000000000000003
> [Thu Dec 17 11:35:37 2020] R10: ffff9866433336c3 R11: 0000000000000000
> R12: 0000000000000000
> [Thu Dec 17 11:35:37 2020] R13: ffff8959e2373428 R14: ffff8959e24b3d40
> R15: 0000000000000008
> [Thu Dec 17 11:35:37 2020] FS: 00007fa878948740(0000)
> GS:ffff895cede00000(0000) knlGS:0000000000000000
> [Thu Dec 17 11:35:37 2020] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [Thu Dec 17 11:35:37 2020] CR2: 0000000000000038 CR3: 0000000121f34001
> CR4: 00000000003706f0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: BPF Kernel OOPS - NULL pointer dereference
2021-01-06 10:14 ` Gilad Reti
@ 2021-01-06 17:26 ` Alexei Starovoitov
2021-01-06 21:02 ` KP Singh
0 siblings, 1 reply; 5+ messages in thread
From: Alexei Starovoitov @ 2021-01-06 17:26 UTC (permalink / raw)
To: Gilad Reti, KP Singh; +Cc: bpf
On Wed, Jan 6, 2021 at 2:18 AM Gilad Reti <gilad.reti@gmail.com> wrote:
>
> Hello all,
>
> Hope you had a great new year vacation.
>
> Sorry for bumping up the thread, but I believe that it is an important one.
Thanks for bumping.
KP, please take a look.
> On Sun, Dec 20, 2020 at 12:16 PM Gilad Reti <gilad.reti@gmail.com> wrote:
> >
> > Hello there,
> >
> > During experimenting with the new inode local storage I have stumbled
> > across an invalid pointer dereference that passed through the
> > verifier.
> >
> > After investigating, I think that it happens in the
> > bpf_inode_storage_get helper, and in particular in the following line:
> > https://elixir.bootlin.com/linux/v5.10.1/source/include/linux/bpf_lsm.h#L32
> >
> > I have a single bpf lsm probe in the security_inode_rename probe, and
> > I have called bpf_inode_storage_get on the inode of the "new_dentry"
> > argument. This inode may be null in cases where renameat(2) is called
> > to move a file from one path to a new path which didn't exist before.
> >
> > As can be seen, inode is dereferenced without first checking that it
> > is not NULL.
> > I don't know what should be the correct behavior, but I believe that
> > either the helper should check the validity of passed pointers, or the
> > verifier should treat fields of BTF pointers (PTR_TO_BTF_ID) as
> > PTR_TO_BTF_ID_OR_NULL.
> >
> > I am attaching a minimal program example, along with the kernel demsg
> > output. To reproduce, load the probe and run "mv file1 file2" where
> > file2 does not exist.
> >
> > Thanks,
> > Gilad Reti
> >
> > inode_oops.c:
> >
> > // SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> > /* Copyright (c) 2020 Facebook */
> > #include <argp.h>
> > #include <signal.h>
> > #include <stdio.h>
> > #include <time.h>
> > #include <unistd.h>
> > #include <sys/resource.h>
> > #include <bpf/libbpf.h>
> >
> > #include "inode_oops.skel.h"
> >
> > static volatile bool exiting = false;
> >
> > static void sig_handler(int sig)
> > {
> > exiting = true;
> > }
> >
> > int main(int argc, char **argv)
> > {
> > struct inode_oops_bpf *skel;
> >
> > int err;
> >
> > /* Cleaner handling of Ctrl-C */
> > signal(SIGINT, sig_handler);
> > signal(SIGTERM, sig_handler);
> >
> > /* Load and verify BPF application */
> > skel = inode_oops_bpf__open_and_load();
> > if (!skel)
> > {
> > fprintf(stderr, "Failed to open and load BPF skeleton\n");
> > goto cleanup;
> > }
> >
> > /* Attach tracepoints */
> > err = inode_oops_bpf__attach(skel);
> > if (err)
> > {
> > fprintf(stderr, "Failed to attach BPF skeleton\n");
> > goto cleanup;
> > }
> >
> > while (!exiting)
> > {
> > }
> > cleanup:
> > /* Clean up */
> > inode_oops_bpf__destroy(skel);
> >
> > return err < 0 ? -err : 0;
> > }
> >
> > inode_oops.bpf.c
> >
> > #include "vmlinux.h"
> > #include <bpf/bpf_core_read.h>
> > #include <bpf/bpf_helpers.h>
> > #include <bpf/bpf_tracing.h>
> >
> > struct dummy_storage
> > {
> > __u32 value;
> > };
> >
> > struct
> > {
> > __uint(type, BPF_MAP_TYPE_INODE_STORAGE);
> > __uint(map_flags, BPF_F_NO_PREALLOC);
> > __type(key, int);
> > __type(value, struct dummy_storage);
> > } inode_storage_map SEC(".maps");
> >
> > SEC("lsm/inode_rename")
> > int BPF_PROG(inode_rename, struct inode *old_dir, struct dentry *old_dentry,
> > struct inode *new_dir, struct dentry *new_dentry,
> > unsigned int flags)
> > {
> >
> > bpf_inode_storage_get(&inode_storage_map, new_dentry->d_inode, 0,
> > BPF_LOCAL_STORAGE_GET_F_CREATE);
> >
> > return 0;
> > }
> >
> > Dmesg -T:
> >
> > [Thu Dec 17 11:35:37 2020] BUG: kernel NULL pointer dereference,
> > address: 0000000000000038
> > [Thu Dec 17 11:35:37 2020] #PF: supervisor read access in kernel mode
> > [Thu Dec 17 11:35:37 2020] #PF: error_code(0x0000) - not-present page
> > [Thu Dec 17 11:35:37 2020] PGD 0 P4D 0
> > [Thu Dec 17 11:35:37 2020] Oops: 0000 [#1] SMP PTI
> > [Thu Dec 17 11:35:37 2020] CPU: 0 PID: 4437 Comm: bash Not tainted 5.10.0 #17
> > [Thu Dec 17 11:35:37 2020] Hardware name: VMware, Inc. VMware Virtual
> > Platform/440BX Desktop Reference Platform, BIOS 6.00 02/27/2020
> > [Thu Dec 17 11:35:37 2020] RIP: 0010:bpf_inode_storage_get+0x1f/0xa0
> > [Thu Dec 17 11:35:37 2020] Code: 65 f8 c9 c3 0f 1f 80 00 00 00 00 0f
> > 1f 44 00 00 48 f7 c1 fe ff ff ff 74 03 31 c0 c3 55 48 89 e5 41 56 41
> > 55 41 54 49 89 f4 53 <48> 8b 46 38 48 85 c0 74 41 49 89 d6 48 63 15 e6
> > d7 28 01 48 01 d0
> > [Thu Dec 17 11:35:37 2020] RSP: 0018:ffff9866433335f8 EFLAGS: 00010246
> > [Thu Dec 17 11:35:37 2020] RAX: 0000000000000000 RBX: 0000000000000001
> > RCX: 0000000000000000
> > [Thu Dec 17 11:35:37 2020] RDX: 0000000000000000 RSI: 0000000000000000
> > RDI: ffff8959e2db9e00
> > [Thu Dec 17 11:35:37 2020] RBP: ffff986643333618 R08: 0000000000000007
> > R09: 0000000000000003
> > [Thu Dec 17 11:35:37 2020] R10: ffff9866433336c3 R11: 0000000000000000
> > R12: 0000000000000000
> > [Thu Dec 17 11:35:37 2020] R13: ffff8959e2373428 R14: ffff8959e24b3d40
> > R15: 0000000000000008
> > [Thu Dec 17 11:35:37 2020] FS: 00007fa878948740(0000)
> > GS:ffff895cede00000(0000) knlGS:0000000000000000
> > [Thu Dec 17 11:35:37 2020] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [Thu Dec 17 11:35:37 2020] CR2: 0000000000000038 CR3: 0000000121f34001
> > CR4: 00000000003706f0
> > [Thu Dec 17 11:35:37 2020] Call Trace:
> > [Thu Dec 17 11:35:37 2020] bpf_prog_f2cb36e361a3c858_inode_rename+0x88c/0xf38
> > [Thu Dec 17 11:35:37 2020] ? security_capable+0x3d/0x60
> > [Thu Dec 17 11:35:37 2020] ? from_kgid+0x12/0x20
> > [Thu Dec 17 11:35:37 2020] ? capable_wrt_inode_uidgid+0x6f/0x80
> > [Thu Dec 17 11:35:37 2020] bpf_trampoline_22333+0x30/0x1000
> > [Thu Dec 17 11:35:37 2020] bpf_lsm_inode_rename+0x5/0x10
> > [Thu Dec 17 11:35:37 2020] ? security_inode_rename+0x88/0xb0
> > [Thu Dec 17 11:35:37 2020] vfs_rename+0x11b/0xb60
> > [Thu Dec 17 11:35:37 2020] ovl_copy_up_one+0x461/0xfc0 [overlay]
> > [Thu Dec 17 11:35:37 2020] ? remove_wait_queue+0x47/0x50
> > [Thu Dec 17 11:35:37 2020] ? fput+0x13/0x20
> > [Thu Dec 17 11:35:37 2020] ? poll_freewait+0x4a/0xa0
> > [Thu Dec 17 11:35:37 2020] ovl_copy_up_flags+0xb0/0xf0 [overlay]
> > [Thu Dec 17 11:35:37 2020] ovl_copy_up+0x10/0x20 [overlay]
> > [Thu Dec 17 11:35:37 2020] ovl_create_or_link+0x40/0x8c0 [overlay]
> > [Thu Dec 17 11:35:37 2020] ? security_inode_alloc+0x4b/0x90
> > [Thu Dec 17 11:35:37 2020] ? inode_init_always+0x137/0x210
> > [Thu Dec 17 11:35:37 2020] ? alloc_inode+0x35/0xc0
> > [Thu Dec 17 11:35:37 2020] ? new_inode+0x74/0xc0
> > [Thu Dec 17 11:35:37 2020] ovl_create_object+0xe1/0x110 [overlay]
> > [Thu Dec 17 11:35:37 2020] ovl_create+0x23/0x30 [overlay]
> > [Thu Dec 17 11:35:37 2020] path_openat+0xdec/0x1130
> > [Thu Dec 17 11:35:37 2020] ? __check_object_size+0x13f/0x150
> > [Thu Dec 17 11:35:37 2020] do_filp_open+0x8c/0x130
> > [Thu Dec 17 11:35:37 2020] ? __check_object_size+0x13f/0x150
> > [Thu Dec 17 11:35:37 2020] do_sys_openat2+0x9b/0x150
> > [Thu Dec 17 11:35:37 2020] __x64_sys_openat+0x56/0x90
> > [Thu Dec 17 11:35:37 2020] do_syscall_64+0x38/0x90
> > [Thu Dec 17 11:35:37 2020] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > [Thu Dec 17 11:35:37 2020] RIP: 0033:0x7fa878018d5e
> > [Thu Dec 17 11:35:37 2020] Code: 25 00 00 41 00 3d 00 00 41 00 74 48
> > 48 8d 05 91 0c 2e 00 8b 00 85 c0 75 69 89 f2 b8 01 01 00 00 48 89 fe
> > bf 9c ff ff ff 0f 05 <48> 3d 00 f0 ff ff 0f 87 a6 00 00 00 48 8b 4c 24
> > 28 64 48 33 0c 25
> > [Thu Dec 17 11:35:37 2020] RSP: 002b:00007ffead706e80 EFLAGS: 00000246
> > ORIG_RAX: 0000000000000101
> > [Thu Dec 17 11:35:37 2020] RAX: ffffffffffffffda RBX: 0000557d910892e0
> > RCX: 00007fa878018d5e
> > [Thu Dec 17 11:35:37 2020] RDX: 0000000000000241 RSI: 0000557d910892e0
> > RDI: 00000000ffffff9c
> > [Thu Dec 17 11:35:37 2020] RBP: 0000000000000001 R08: 00007fa8782f68b0
> > R09: 00007fa878948740
> > [Thu Dec 17 11:35:37 2020] R10: 0000000000000180 R11: 0000000000000246
> > R12: 0000000000000000
> > [Thu Dec 17 11:35:37 2020] R13: 00007ffead707150 R14: 0000557d8f8cfcc0
> > R15: 0000000000000000
> > [Thu Dec 17 11:35:37 2020] Modules linked in: xt_nat veth vxlan
> > ip6_udp_tunnel udp_tunnel xt_policy xt_mark xt_u32 xt_tcpudp
> > xt_conntrack xt_MASQUERADE nf_conntrack_netlink xfrm_user xfrm_algo
> > nft_counter xt_addrtype nft_compat nft_chain_nat nf_nat nf_conntrack
> > nf_defrag_ipv6 nf_defrag_ipv4 nf_tables libcrc32c nfnetlink
> > br_netfilter bridge stp llc intel_rapl_msr intel_rapl_common
> > crct10dif_pclmul ghash_clmulni_intel snd_ens1371 overlay aesni_intel
> > crypto_simd snd_ac97_codec vmw_balloon cryptd vsock_loopback gameport
> > vmw_vsock_virtio_transport_common ac97_bus glue_helper snd_pcm
> > vmw_vsock_vmci_transport vsock rapl snd_seq_midi snd_seq_midi_event
> > snd_rawmidi snd_seq joydev input_leds serio_raw snd_seq_device
> > snd_timer binfmt_misc snd soundcore vmw_vmci mac_hid sch_fq_codel
> > vmwgfx ttm drm_kms_helper cec rc_core drm fb_sys_fops syscopyarea
> > sysfillrect sysimgblt parport_pc ppdev lp parport ip_tables x_tables
> > autofs4 hid_generic usbhid hid psmouse mptspi crc32_pclmul mptscsih
> > mptbase e1000 i2c_piix4 pata_acpi
> > [Thu Dec 17 11:35:37 2020] ahci libahci scsi_transport_spi
> > [Thu Dec 17 11:35:37 2020] CR2: 0000000000000038
> > [Thu Dec 17 11:35:37 2020] ---[ end trace fe83c4e3d6415215 ]---
> > [Thu Dec 17 11:35:37 2020] RIP: 0010:bpf_inode_storage_get+0x1f/0xa0
> > [Thu Dec 17 11:35:37 2020] Code: 65 f8 c9 c3 0f 1f 80 00 00 00 00 0f
> > 1f 44 00 00 48 f7 c1 fe ff ff ff 74 03 31 c0 c3 55 48 89 e5 41 56 41
> > 55 41 54 49 89 f4 53 <48> 8b 46 38 48 85 c0 74 41 49 89 d6 48 63 15 e6
> > d7 28 01 48 01 d0
> > [Thu Dec 17 11:35:37 2020] RSP: 0018:ffff9866433335f8 EFLAGS: 00010246
> > [Thu Dec 17 11:35:37 2020] RAX: 0000000000000000 RBX: 0000000000000001
> > RCX: 0000000000000000
> > [Thu Dec 17 11:35:37 2020] RDX: 0000000000000000 RSI: 0000000000000000
> > RDI: ffff8959e2db9e00
> > [Thu Dec 17 11:35:37 2020] RBP: ffff986643333618 R08: 0000000000000007
> > R09: 0000000000000003
> > [Thu Dec 17 11:35:37 2020] R10: ffff9866433336c3 R11: 0000000000000000
> > R12: 0000000000000000
> > [Thu Dec 17 11:35:37 2020] R13: ffff8959e2373428 R14: ffff8959e24b3d40
> > R15: 0000000000000008
> > [Thu Dec 17 11:35:37 2020] FS: 00007fa878948740(0000)
> > GS:ffff895cede00000(0000) knlGS:0000000000000000
> > [Thu Dec 17 11:35:37 2020] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [Thu Dec 17 11:35:37 2020] CR2: 0000000000000038 CR3: 0000000121f34001
> > CR4: 00000000003706f0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: BPF Kernel OOPS - NULL pointer dereference
2021-01-06 17:26 ` Alexei Starovoitov
@ 2021-01-06 21:02 ` KP Singh
2021-01-07 2:51 ` KP Singh
0 siblings, 1 reply; 5+ messages in thread
From: KP Singh @ 2021-01-06 21:02 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Gilad Reti, KP Singh, bpf
On Wed, Jan 6, 2021 at 6:27 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Jan 6, 2021 at 2:18 AM Gilad Reti <gilad.reti@gmail.com> wrote:
> >
> > Hello all,
> >
> > Hope you had a great new year vacation.
> >
> > Sorry for bumping up the thread, but I believe that it is an important one.
>
> Thanks for bumping.
> KP, please take a look.
>
> > On Sun, Dec 20, 2020 at 12:16 PM Gilad Reti <gilad.reti@gmail.com> wrote:
> > >
> > > Hello there,
> > >
> > > During experimenting with the new inode local storage I have stumbled
> > > across an invalid pointer dereference that passed through the
> > > verifier.
> > >
> > > After investigating, I think that it happens in the
> > > bpf_inode_storage_get helper, and in particular in the following line:
> > > https://elixir.bootlin.com/linux/v5.10.1/source/include/linux/bpf_lsm.h#L32
> > >
> > > I have a single bpf lsm probe in the security_inode_rename probe, and
> > > I have called bpf_inode_storage_get on the inode of the "new_dentry"
> > > argument. This inode may be null in cases where renameat(2) is called
> > > to move a file from one path to a new path which didn't exist before.
> > >
> > > As can be seen, inode is dereferenced without first checking that it
> > > is not NULL.
> > > I don't know what should be the correct behavior, but I believe that
> > > either the helper should check the validity of passed pointers, or the
> > > verifier should treat fields of BTF pointers (PTR_TO_BTF_ID) as
> > > PTR_TO_BTF_ID_OR_NULL.
Thanks for adding me, I missed responding to this over the winter holidays.
In this case this should indeed be PTR_TO_BTF_ID_OR_NULL. I will send in a
fix.
> > >
> > > I am attaching a minimal program example, along with the kernel demsg
> > > output. To reproduce, load the probe and run "mv file1 file2" where
> > > file2 does not exist.
Thanks for sending the repro program as well! Really appreciate it!
> > >
> > > Thanks,
> > > Gilad Reti
> > >
> > > inode_oops.c:
> > >
> > > // SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
[...]
> > > [Thu Dec 17 11:35:37 2020] FS: 00007fa878948740(0000)
> > > GS:ffff895cede00000(0000) knlGS:0000000000000000
> > > [Thu Dec 17 11:35:37 2020] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > [Thu Dec 17 11:35:37 2020] CR2: 0000000000000038 CR3: 0000000121f34001
> > > CR4: 00000000003706f0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: BPF Kernel OOPS - NULL pointer dereference
2021-01-06 21:02 ` KP Singh
@ 2021-01-07 2:51 ` KP Singh
0 siblings, 0 replies; 5+ messages in thread
From: KP Singh @ 2021-01-07 2:51 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Gilad Reti, KP Singh, bpf
On Wed, Jan 6, 2021 at 10:02 PM KP Singh <kpsingh@kernel.org> wrote:
>
> On Wed, Jan 6, 2021 at 6:27 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Wed, Jan 6, 2021 at 2:18 AM Gilad Reti <gilad.reti@gmail.com> wrote:
> > >
> > > Hello all,
> > >
> > > Hope you had a great new year vacation.
> > >
> > > Sorry for bumping up the thread, but I believe that it is an important one.
> >
> > Thanks for bumping.
> > KP, please take a look.
> >
> > > On Sun, Dec 20, 2020 at 12:16 PM Gilad Reti <gilad.reti@gmail.com> wrote:
> > > >
> > > > Hello there,
> > > >
> > > > During experimenting with the new inode local storage I have stumbled
> > > > across an invalid pointer dereference that passed through the
> > > > verifier.
> > > >
> > > > After investigating, I think that it happens in the
> > > > bpf_inode_storage_get helper, and in particular in the following line:
> > > > https://elixir.bootlin.com/linux/v5.10.1/source/include/linux/bpf_lsm.h#L32
> > > >
> > > > I have a single bpf lsm probe in the security_inode_rename probe, and
> > > > I have called bpf_inode_storage_get on the inode of the "new_dentry"
> > > > argument. This inode may be null in cases where renameat(2) is called
> > > > to move a file from one path to a new path which didn't exist before.
> > > >
> > > > As can be seen, inode is dereferenced without first checking that it
> > > > is not NULL.
> > > > I don't know what should be the correct behavior, but I believe that
> > > > either the helper should check the validity of passed pointers, or the
> > > > verifier should treat fields of BTF pointers (PTR_TO_BTF_ID) as
> > > > PTR_TO_BTF_ID_OR_NULL.
>
> Thanks for adding me, I missed responding to this over the winter holidays.
>
> In this case this should indeed be PTR_TO_BTF_ID_OR_NULL. I will send in a
Okay so I dug in a little more...We don't really need this
"PTR_TO_BTF_ID points to a kernel struct that does not need to be null
checked by the BPF program. This does not imply the pointer is _not_
null and in practice this can easily be a null pointer when reading
pointer chains. The assumption is program context will handle null
pointer dereference typically via fault handling. The verifier must
keep this in mind and can make no assumptions about null or non-null
when doing branch analysis.
Further, when passed into helpers the helpers can not, without
additional context, assume the value is non-null."
So all we need to do is to check the null-ness of the pointer in the
helper code, the helpers cannot simply assume that the pointer
will be non NULL.
This does the trick, I will send in a patch tomorrow:
diff --git a/kernel/bpf/bpf_inode_storage.c b/kernel/bpf/bpf_inode_storage.c
index 6edff97ad594..92a59b283316 100644
--- a/kernel/bpf/bpf_inode_storage.c
+++ b/kernel/bpf/bpf_inode_storage.c
@@ -168,6 +168,9 @@ BPF_CALL_4(bpf_inode_storage_get, struct bpf_map
*, map, struct inode *, inode,
{
struct bpf_local_storage_data *sdata;
+ if (!inode)
+ return (unsigned long)NULL;
+
if (flags & ~(BPF_LOCAL_STORAGE_GET_F_CREATE))
return (unsigned long)NULL;
@@ -200,6 +203,9 @@ BPF_CALL_4(bpf_inode_storage_get, struct bpf_map
*, map, struct inode *, inode,
BPF_CALL_2(bpf_inode_storage_delete,
struct bpf_map *, map, struct inode *, inode)
{
+ if (!inode)
+ return -EINVAL;
+
/* This helper must only called from where the inode is gurranteed
* to have a refcount and cannot be freed.
*/
- KP
> fix.
>
> > > >
> > > > I am attaching a minimal program example, along with the kernel demsg
> > > > output. To reproduce, load the probe and run "mv file1 file2" where
> > > > file2 does not exist.
>
> Thanks for sending the repro program as well! Really appreciate it!
>
> > > >
> > > > Thanks,
> > > > Gilad Reti
> > > >
> > > > inode_oops.c:
> > > >
> > > > // SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
>
> [...]
>
> > > > [Thu Dec 17 11:35:37 2020] FS: 00007fa878948740(0000)
> > > > GS:ffff895cede00000(0000) knlGS:0000000000000000
> > > > [Thu Dec 17 11:35:37 2020] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > [Thu Dec 17 11:35:37 2020] CR2: 0000000000000038 CR3: 0000000121f34001
> > > > CR4: 00000000003706f0
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-07 2:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-20 10:16 BPF Kernel OOPS - NULL pointer dereference Gilad Reti
2021-01-06 10:14 ` Gilad Reti
2021-01-06 17:26 ` Alexei Starovoitov
2021-01-06 21:02 ` KP Singh
2021-01-07 2:51 ` KP Singh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).