Re: [PATCH bpf-next 1/2] bpf: Add bitwise tracking for BPF_END

[Eduard Zingerman <eddyz87@gmail.com>]

On Tue, 2026-02-03 at 18:17 +0800, Tianci Cao wrote:
> Hi Eduard,
> 
> Thank you for the review and the ack.
> 
> On 2/3/26 4:03 AM, Eduard Zingerman wrote:
> > > -	/* ALU32 ops are zero extended into 64bit register */
> > > -	if (alu32)
> > > +	/*
> > > +	 * ALU32 ops are zero extended into 64bit register.
> > > +	 * BPF_END is already handled inside the helper (truncation),
> > > +	 * so skip zext here.
> > > +	 */
> > > +	if (alu32 && opcode != BPF_END)
> > 
> > Nit: zext after scalar_byte_swap() won't change anything, right?
> >      If so, I'd just avoid the special case and skip 'opcode != BPF_END'.
> 
> Actually, for BPF_END, the alu32 flag may mislead regarding the actual data width being processed.
> The operand size for BPF_END is determined by insn->imm , not by the opcode class. 
> So if we simply rely on if (alu32) to perform zext_32_to_64(), we might encounter an unsound state 
> in the following cases:
> 
> * le64 : opcode=0xd4 (BPF_END | BPF_ALU | BPF_TO_LE)
> * be64 : opcode=0xdc (BPF_END | BPF_ALU | BPF_TO_BE)
> 
> These instructions belong to BPF_ALU, so alu32 is true. However, it is a 64-bit operation. If we 
> perform zext_32_to_64() after scalar_byte_swap(), the verifier would incorrectly zero-out the 
> upper 32 bits of a 64-bit result, which is incorrect and unsound.
> 
> Since scalar_byte_swap() already handles truncation and zero-extension internally based on the 
> precise ' insn->imm ', it's safer to skip the generic alu32 zext for BPF_END altogether.
> 
> If you have any more questions, please feel free to reach out.

Hm, makes sense, thank you for explaining.