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@xxxxxxxxxx/
>
> Reported-by: syzbot+ad9ec60c8eaf69e6f99c@xxxxxxxxxxxxxxxxxxxxxxxxx
> Fixes: 1f1e864b6555 ("bpf: Handle sign-extenstin ctx member accesses")
> Signed-off-by: Yonghong Song <yonghong.song@xxxxxxxxx>
> ---
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@xxxxxxxxx>
