Re: [PATCH bpf-next 1/2] bpf: Fail verification for sign-extension of packet data/data_end/data_meta

[Eduard Zingerman <eddyz87@gmail.com>]

On Mon, 2024-07-15 at 13:18 -0700, Yonghong Song wrote:
> syzbot reported a kernel crash due to
>   commit 1f1e864b6555 ("bpf: Handle sign-extenstin ctx member accesses").
> The reason is due to sign-extension of 32-bit load for
> packet data/data_end/data_meta uapi field.
> 
> The original code looks like:
>         r2 = *(s32 *)(r1 + 76) /* load __sk_buff->data */
>         r3 = *(u32 *)(r1 + 80) /* load __sk_buff->data_end */
>         r0 = r2
>         r0 += 8
>         if r3 > r0 goto +1
>         ...
> Note that __sk_buff->data load has 32-bit sign extension.

[...]

> To fix this issue for case
>   r2 = *(s32 *)(r1 + 76) /* load __sk_buff->data */
> this patch added additional checking in is_valid_access() callback
> function for packet data/data_end/data_meta access. If those accesses
> are with sign-extenstion, the verification will fail.
> 
>   [1] https://lore.kernel.org/bpf/000000000000c90eee061d236d37@google.com/
> 
> Reported-by: syzbot+ad9ec60c8eaf69e6f99c@syzkaller.appspotmail.com
> Fixes: 1f1e864b6555 ("bpf: Handle sign-extenstin ctx member accesses")
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---

I looked through all context types and seems like only two types
identified in this patch use u32 values to obtain pointers:
- struct xdp_md       fields: data, data_end, data_meta
- struct __sk_buff    fields: data, data_end, data_meta

Double checked all locations where access to the above fields is
verified, every location is covered by is_ldsx check.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>