Re: [PATCH bpf-next V3] bpf: avoid UB in usages of the __imm_insn macro

[Yonghong Song <yonghong.song@linux.dev>]

On 5/8/24 3:35 AM, Jose E. Marchesi wrote:
> [Changes from V2:
>   - no-strict-aliasing is only applied when building with GCC.
>   - cpumask_failure.c is excluded, as it doesn't use __imm_insn.]
>
> The __imm_insn macro is defined in bpf_misc.h as:
>
>    #define __imm_insn(name, expr) [name]"i"(*(long *)&(expr))
>
> This may lead to type-punning and strict aliasing rules violations in
> it's typical usage where the address of a struct bpf_insn is passed as
> expr, like in:
>
>    __imm_insn(st_mem,
>               BPF_ST_MEM(BPF_W, BPF_REG_1, offsetof(struct __sk_buff, mark), 42))
>
> Where:
>
>    #define BPF_ST_MEM(SIZE, DST, OFF, IMM)				\
> 	((struct bpf_insn) {					\
> 		.code  = BPF_ST | BPF_SIZE(SIZE) | BPF_MEM,	\
> 		.dst_reg = DST,					\
> 		.src_reg = 0,					\
> 		.off   = OFF,					\
> 		.imm   = IMM })
>
> In all the actual instances of this in the BPF selftests the value is
> fed to a volatile asm statement as soon as it gets read from memory,
> and thus it is unlikely anti-aliasing rules breakage may lead to
> misguided optimizations.
>
> However, GCC detects the potential problem (indirectly) by issuing a
> warning stating that a temporary <Uxxxxxx> is used uninitialized,
> where the temporary corresponds to the memory read by *(long *).
>
> This patch adds -fno-strict-aliasing to the compilation flags of the
> particular selftests that do type punning via __imm_insn, only for
> GCC.
>
> Tested in master bpf-next.
> No regressions.
>
> Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>
> Cc: david.faust@oracle.com
> Cc: cupertino.miranda@oracle.com
> Cc: Yonghong Song <yonghong.song@linux.dev>
> Cc: Eduard Zingerman <eddyz87@gmail.com>

Acked-by: Yonghong Song <yonghong.song@linux.dev>