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  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cf475fe0-9961-d768-fae3-04f640855dab@iogearbox.net>
Date:   Tue, 3 Jan 2023 16:41:58 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     xxmy@...ragraf.org, bpf@...r.kernel.org, netdev@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.or, loongarch@...ts.linux.dev,
        linux-mips@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        linux-riscv@...ts.infradead.org, linux-s390@...r.kernel.org,
        sparclinux@...r.kernel.org
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Stanislav Fomichev <sdf@...gle.com>,
        Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
        Hou Tao <houtao1@...wei.com>
Subject: Re: [bpf-next v1] bpf: drop deprecated bpf_jit_enable == 2

On 1/3/23 2:24 PM, xxmy@...ragraf.org wrote:
> From: Tonghao Zhang <xxmy@...ragraf.org>
> 
> The x86_64 can't dump the valid insn in this way. A test BPF prog
> which include subprog:
> 
> $ llvm-objdump -d subprog.o
> Disassembly of section .text:
> 0000000000000000 <subprog>:
>         0:	18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00	r1 = 29114459903653235 ll
>         2:	7b 1a f8 ff 00 00 00 00	*(u64 *)(r10 - 8) = r1
>         3:	bf a1 00 00 00 00 00 00	r1 = r10
>         4:	07 01 00 00 f8 ff ff ff	r1 += -8
>         5:	b7 02 00 00 08 00 00 00	r2 = 8
>         6:	85 00 00 00 06 00 00 00	call 6
>         7:	95 00 00 00 00 00 00 00	exit
> Disassembly of section raw_tp/sys_enter:
> 0000000000000000 <entry>:
>         0:	85 10 00 00 ff ff ff ff	call -1
>         1:	b7 00 00 00 00 00 00 00	r0 = 0
>         2:	95 00 00 00 00 00 00 00	exit
> 
> kernel print message:
> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c from=kprobe-load pid=1643
> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
> [  580.782568] JIT code: 00000030: cc cc cc
> 
> $ bpf_jit_disasm
> 51 bytes emitted from JIT compiler (pass:3, flen:8)
> ffffffffa000c20c + <x>:
>     0:	int3
>     1:	int3
>     2:	int3
>     3:	int3
>     4:	int3
>     5:	int3
>     ...
> 
> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to header
> and then image/insn is valid. BTW, we can use the "bpftool prog dump" JITed instructions.
> 
> * clean up the doc
> * remove bpf_jit_disasm tool
> * set bpf_jit_enable only 0 or 1.
> 
> Signed-off-by: Tonghao Zhang <xxmy@...ragraf.org>
> Suggested-by: Alexei Starovoitov <ast@...nel.org>
> Cc: Alexei Starovoitov <ast@...nel.org>
> Cc: Daniel Borkmann <daniel@...earbox.net>
> Cc: Andrii Nakryiko <andrii@...nel.org>
> Cc: Martin KaFai Lau <martin.lau@...ux.dev>
> Cc: Song Liu <song@...nel.org>
> Cc: Yonghong Song <yhs@...com>
> Cc: John Fastabend <john.fastabend@...il.com>
> Cc: KP Singh <kpsingh@...nel.org>
> Cc: Stanislav Fomichev <sdf@...gle.com>
> Cc: Hao Luo <haoluo@...gle.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Hou Tao <houtao1@...wei.com>
> ---
>   Documentation/admin-guide/sysctl/net.rst |   2 +-
>   Documentation/networking/filter.rst      |  98 +------
>   arch/arm/net/bpf_jit_32.c                |   4 -
>   arch/arm64/net/bpf_jit_comp.c            |   4 -
>   arch/loongarch/net/bpf_jit.c             |   4 -
>   arch/mips/net/bpf_jit_comp.c             |   3 -
>   arch/powerpc/net/bpf_jit_comp.c          |  11 -
>   arch/riscv/net/bpf_jit_core.c            |   3 -
>   arch/s390/net/bpf_jit_comp.c             |   4 -
>   arch/sparc/net/bpf_jit_comp_32.c         |   3 -
>   arch/sparc/net/bpf_jit_comp_64.c         |  13 -
>   arch/x86/net/bpf_jit_comp.c              |   3 -
>   arch/x86/net/bpf_jit_comp32.c            |   3 -
>   net/core/sysctl_net_core.c               |  14 +-
>   tools/bpf/.gitignore                     |   1 -
>   tools/bpf/Makefile                       |  10 +-
>   tools/bpf/bpf_jit_disasm.c               | 332 -----------------------
>   17 files changed, 8 insertions(+), 504 deletions(-)
>   delete mode 100644 tools/bpf/bpf_jit_disasm.c
> 
> diff --git a/Documentation/admin-guide/sysctl/net.rst b/Documentation/admin-guide/sysctl/net.rst
> index 6394f5dc2303..45d3d965276c 100644
> --- a/Documentation/admin-guide/sysctl/net.rst
> +++ b/Documentation/admin-guide/sysctl/net.rst
> @@ -87,7 +87,7 @@ Values:
>   
>   	- 0 - disable the JIT (default value)
>   	- 1 - enable the JIT
> -	- 2 - enable the JIT and ask the compiler to emit traces on kernel log.
> +	- 2 - deprecated in linux 6.2

I'd make it more clear in the docs and reword it as follows (also, it'll be in 6.3, not 6.2):

	- 2 - enable the JIT and ask the compiler to emit traces on kernel log.
	      (deprecated since v6.3, use ``bpftool prog dump jited id <id>`` instead)

>   
>   bpf_jit_harden
>   --------------
[...]
> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index 5b1ce656baa1..731a2eb0f68e 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c
> @@ -275,16 +275,8 @@ static int proc_dointvec_minmax_bpf_enable(struct ctl_table *table, int write,
>   
>   	tmp.data = &jit_enable;
>   	ret = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos);
> -	if (write && !ret) {
> -		if (jit_enable < 2 ||
> -		    (jit_enable == 2 && bpf_dump_raw_ok(current_cred()))) {
> -			*(int *)table->data = jit_enable;
> -			if (jit_enable == 2)
> -				pr_warn("bpf_jit_enable = 2 was set! NEVER use this in production, only for JIT debugging!\n");
> -		} else {
> -			ret = -EPERM;
> -		}
> -	}
> +	if (write && !ret)
> +		*(int *)table->data = jit_enable;
>   
>   	if (write && ret && min == max)
>   		pr_info_once("CONFIG_BPF_JIT_ALWAYS_ON is enabled, bpf_jit_enable is permanently set to 1.\n");
> @@ -395,7 +387,7 @@ static struct ctl_table net_core_table[] = {
>   		.extra2		= SYSCTL_ONE,
>   # else
>   		.extra1		= SYSCTL_ZERO,
> -		.extra2		= SYSCTL_TWO,
> +		.extra2		= SYSCTL_ONE,

I'd leave it at SYSCTL_TWO to avoid breakage, just that the semantics of 2 will be the same
as 1 going forward.

>   # endif
>   	},
>   # ifdef CONFIG_HAVE_EBPF_JIT

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ