linux-kernel - Re: [PATCH bpf-next 3/3] bpf: arraymap: Skip boundscheck during inlining when possible

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: <da5e61f8bd7e3e8a7c9317c023cec93219069e59.camel@gmail.com>
Date: Wed, 22 Jan 2025 16:15:53 -0800
From: Eduard Zingerman <eddyz87@...il.com>
To: Daniel Xu <dxu@...uu.xyz>, daniel@...earbox.net, ast@...nel.org, 
	andrii@...nel.org
Cc: martin.lau@...ux.dev, song@...nel.org, yonghong.song@...ux.dev, 
	john.fastabend@...il.com, kpsingh@...nel.org, sdf@...ichev.me,
 haoluo@...gle.com, 	jolsa@...nel.org, bpf@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next 3/3] bpf: arraymap: Skip boundscheck during
 inlining when possible

On Mon, 2025-01-20 at 21:35 -0700, Daniel Xu wrote:

[...]

Hi Daniel,

> @@ -221,11 +221,13 @@ static int array_map_gen_lookup(struct bpf_map *map,
>  
>  	*insn++ = BPF_ALU64_IMM(BPF_ADD, map_ptr, offsetof(struct bpf_array, value));
>  	*insn++ = BPF_LDX_MEM(BPF_W, ret, index, 0);
> -	if (!map->bypass_spec_v1) {
> -		*insn++ = BPF_JMP_IMM(BPF_JGE, ret, map->max_entries, 4);
> -		*insn++ = BPF_ALU32_IMM(BPF_AND, ret, array->index_mask);
> -	} else {
> -		*insn++ = BPF_JMP_IMM(BPF_JGE, ret, map->max_entries, 3);
> +	if (!inbounds) {
> +		if (!map->bypass_spec_v1) {
> +			*insn++ = BPF_JMP_IMM(BPF_JGE, ret, map->max_entries, 4);
> +			*insn++ = BPF_ALU32_IMM(BPF_AND, ret, array->index_mask);
> +		} else {
> +			*insn++ = BPF_JMP_IMM(BPF_JGE, ret, map->max_entries, 3);
> +		}
>  	}
>  
>  	if (is_power_of_2(elem_size)) {

Note that below this hunk there is the following code:

	*insn++ = BPF_JMP_IMM(BPF_JA, 0, 0, 1);
	*insn++ = BPF_MOV64_IMM(ret, 0);
	return insn - insn_buf;

This part becomes redundant after your change. E.g. here is jit
listing for an_array_with_a_32bit_constant_0_no_nullness selftest:

JITED:
=============
func #0:
0:	f3 0f 1e fa                         	endbr64
4:	0f 1f 44 00 00                      	nopl	(%rax,%rax)
9:	0f 1f 00                            	nopl	(%rax)
c:	55                                  	pushq	%rbp
d:	48 89 e5                            	movq	%rsp, %rbp
10:	f3 0f 1e fa                         	endbr64
14:	48 81 ec 08 00 00 00                	subq	$0x8, %rsp
1b:	31 ff                               	xorl	%edi, %edi
1d:	89 7d fc                            	movl	%edi, -0x4(%rbp)
20:	48 89 ee                            	movq	%rbp, %rsi
23:	48 83 c6 fc                         	addq	$-0x4, %rsi
27:	48 bf 00 70 58 06 81 88 ff ff       	movabsq	$-0x777ef9a79000, %rdi
31:	48 81 c7 d8 01 00 00                	addq	$0x1d8, %rdi
38:	8b 46 00                            	movl	(%rsi), %eax
3b:	48 6b c0 30                         	imulq	$0x30, %rax, %rax
3f:	48 01 f8                            	addq	%rdi, %rax
42:	eb 02                               	jmp	L0             //
44:	31 c0                               	xorl	%eax, %eax     // never executed
46:	bf 04 00 00 00                      L0:	movl	$0x4, %edi     //
4b:	89 78 00                            	movl	%edi, (%rax)
4e:	b8 04 00 00 00                      	movl	$0x4, %eax
53:	c9                                  	leave
54:	e9 22 38 50 c3                      	jmp	0xffffffffc350387b

Also note that there are __arch_x86_64 and __jited tags for selftests.
These allow to match against disassembly of the generated binary code.
(See verifier_tailcall_jit.c for an example).
I think it would be good to add a test matching jited code for this feature.

[...]