lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 3 Jan 2019 19:12:05 +0000 From: Martin Lau <kafai@...com> To: Heiko Carstens <heiko.carstens@...ibm.com> CC: Eugene Syromyatnikov <evgsyr@...il.com>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, Yonghong Song <yhs@...com>, Alexei Starovoitov <ast@...nel.org>, "Dmitry V. Levin" <ldv@...linux.org> Subject: Re: "bpf: Improve the info.func_info and info.func_info_rec_size behavior" breaks strace self tests On Thu, Jan 03, 2019 at 12:46:13PM +0100, Heiko Carstens wrote: > Hello, > > the kernel commit 7337224fc150 ("bpf: Improve the info.func_info and > info.func_info_rec_size behavior") breaks one of strace's self tests: > > FAIL: bpf-obj_get_info_by_fd-prog-v.gen The strace's bpf-obj_get_info_by_fd-prog-v test did fail. However, I failed to see how 7337224fc150 broke it. How do you trace down to commit 7337224fc150 and can you share your test output? The failure I can reproduce is EFAULT. It should have been failing as early as "nr_jited_ksyms" is added to "struct bpf_prog_info" in linux/bpf.h. The sys_bpf(BPF_OBJ_GET_INFO_BY_FD, &bpf_prog_get_info_attr, ...) is called in a loop. The bpf_prog_get_info_attr.info object is in size 104 but bpf_prog_get_info_attr.info_len is in size 168. Hence, if the prog is jited, the second iteraton onwards will have nr_jited_ksyms == 1 which is asking the kernel to fill the bpf_prog_get_info_attr.info.jited_ksyms and it is NULL. This fix the strace's test program: diff --git i/tests/bpf-obj_get_info_by_fd.c w/tests/bpf-obj_get_info_by_fd.c index e95afda27b3d..953083720822 100644 --- i/tests/bpf-obj_get_info_by_fd.c +++ w/tests/bpf-obj_get_info_by_fd.c @@ -19,6 +19,7 @@ #include <stdint.h> #include <stdlib.h> #include <unistd.h> +#include <string.h> #include <sys/sysmacros.h> #include <asm/unistd.h> @@ -341,6 +342,7 @@ main(void) size_t old_prog_info_len = PROG_INFO_SZ; for (unsigned int i = 0; i < 4; i++) { + memset(prog_info + 1, 0, PROG_INFO_SZ - sizeof(*prog_info)); prog_info->jited_prog_len = 0; switch (i) { case 1: After the above change: [root@...h-fb-vm1 tests]# ./bpf-obj_get_info_by_fd-prog-v bpf(BPF_MAP_CREATE, {map_type=BPF_MAP_TYPE_ARRAY, key_size=4, value_size=8, max_entries=1, map_flags=0, inner_map_fd=0</dev/null>, map_name="test_map", map_ifindex=0}, 48) = 3<anon_inode:bpf-map> bpf(BPF_PROG_LOAD, {prog_type=BPF_PROG_TYPE_SOCKET_FILTER, insn_cnt=9, insns=[{code=BPF_ALU64|BPF_K|BPF_MOV, dst_reg=BPF_REG_1, src_reg=BPF_REG_0, off=0, imm=0}, {code=BPF_STX|BPF_W|BPF_MEM, dst_reg=BPF_REG_10, src_reg=BPF_REG_1, off=-4, imm=0}, {code=BPF_ALU64|BPF_X|BPF_MOV, dst_reg=BPF_REG_2, src_reg=BPF_REG_10, off=0, imm=0}, {code=BPF_ALU64|BPF_K|BPF_ADD, dst_reg=BPF_REG_2, src_reg=BPF_REG_0, off=0, imm=0xfffffffc}, {code=BPF_LD|BPF_DW|BPF_IMM, dst_reg=BPF_REG_1, src_reg=BPF_REG_1, off=0, imm=0x3}, {code=BPF_LD|BPF_W|BPF_IMM, dst_reg=BPF_REG_0, src_reg=BPF_REG_0, off=0, imm=0}, {code=BPF_JMP|BPF_K|BPF_CALL, dst_reg=BPF_REG_0, src_reg=BPF_REG_0, off=0, imm=0x1}, {code=BPF_ALU64|BPF_K|BPF_MOV, dst_reg=BPF_REG_0, src_reg=BPF_REG_0, off=0, imm=0}, {code=BPF_JMP|BPF_K|BPF_EXIT, dst_reg=BPF_REG_0, src_reg=BPF_REG_0, off=0, imm=0}], license="BSD", log_level=42, log_size=4096, log_buf="", kern_version=KERNEL_VERSION(57005, 192, 222), prog_flags=0, prog_name="test_prog", prog_ifindex=0, expected_attach_type=BPF_CGROUP_INET_INGRESS}, 72) = 4<anon_inode:bpf-prog> bpf(BPF_OBJ_GET_INFO_BY_FD, {info={bpf_fd=3<anon_inode:bpf-map>, info_len=128 => 80, info={type=BPF_MAP_TYPE_ARRAY, id=13, key_size=4, value_size=8, max_entries=1, map_flags=0, name="test_map", ifindex=0, netns_dev=makedev(0, 0), netns_ino=0}}}, 16) = 0 bpf(BPF_OBJ_GET_INFO_BY_FD, {info={bpf_fd=4<anon_inode:bpf-prog>, info_len=168, info={type=BPF_PROG_TYPE_SOCKET_FILTER, id=12, tag="\xda\xbf\x02\x07\xd1\x99\x24\x86", jited_prog_len=0 => 110, jited_prog_insns=NULL, xlated_prog_len=0 => 120, xlated_prog_insns=NULL, load_time=1360454956729, created_by_uid=0, nr_map_ids=0 => 1, map_ids=NULL, name="test_prog", ifindex=0, netns_dev=makedev(0, 0), netns_ino=0}}}, 16) = 0 bpf(BPF_OBJ_GET_INFO_BY_FD, {info={bpf_fd=4<anon_inode:bpf-prog>, info_len=168, info={type=BPF_PROG_TYPE_SOCKET_FILTER, id=12, tag="\xda\xbf\x02\x07\xd1\x99\x24\x86", jited_prog_len=0, jited_prog_insns=NULL, xlated_prog_len=336, xlated_prog_insns=0x7fda42a02000, load_time=1360454956729, created_by_uid=0, nr_map_ids=2, map_ids=0x7fda429f5000, name="test_prog", ifindex=0, netns_dev=makedev(0, 0), netns_ino=0}}}, 16) = -1 EFAULT (Bad address) bpf(BPF_OBJ_GET_INFO_BY_FD, {info={bpf_fd=4<anon_inode:bpf-prog>, info_len=168, info={type=BPF_PROG_TYPE_SOCKET_FILTER, id=12, tag="\xda\xbf\x02\x07\xd1\x99\x24\x86", jited_prog_len=0 => 110, jited_prog_insns=NULL, xlated_prog_len=0 => 120, xlated_prog_insns=[], load_time=1360454956729, created_by_uid=0, nr_map_ids=0 => 1, map_ids=[], name="test_prog", ifindex=0, netns_dev=makedev(0, 0), netns_ino=0}}}, 16) = 0 bpf(BPF_OBJ_GET_INFO_BY_FD, {info={bpf_fd=4<anon_inode:bpf-prog>, info_len=168, info={type=BPF_PROG_TYPE_SOCKET_FILTER, id=12, tag="\xda\xbf\x02\x07\xd1\x99\x24\x86", jited_prog_len=0 => 110, jited_prog_insns=NULL, xlated_prog_len=0 => 120, xlated_prog_insns=[], load_time=1360454956729, created_by_uid=0, nr_map_ids=2 => 1, map_ids=[13], name="test_prog", ifindex=0, netns_dev=makedev(0, 0), netns_ino=0}}}, 16) = 0 +++ exited with 0 +++ > > Looking into the kernel commit, it seems that the user space visible > uapi change is intentional; even though it might break existing user > space. > > To reproduce: > > git clone https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_strace_strace.git&d=DwIDAg&c=5VD0RTtNlTh3ycd41b3MUw&r=VQnoQ7LvghIj0gVEaiQSUw&m=0KV2Bnx31aCm6v7x7mYMZ26wy_rnRwk6QrpXEXP0CIE&s=c7S2IRhn_p6FF1Z2HrhC8mAMvncVDYMPpLxhvftDlV8&e= > cd strace > ./bootstrap > ./configure > make -j $(nproc) > cd tests > make -j $(nproc) check >
Powered by blists - more mailing lists