On Tue, 2024-12-03 at 17:26 +0100, Nick Zavaritsky wrote:
> Hi,
>
> Calls to helpers such as bpf_skb_pull_data, are supposed to invalidate
> all prior checks on packet pointers.
>
> I noticed that if I wrap a call to bpf_skb_pull_data in a function with
> global linkage, pointers checked prior to the call are still considered
> valid after the call. The program is accepted on 6.8 and 6.13-rc1.
>
> I'm curious if it is by design and if not, if it is a known issue.
> Please find the program below.
>
> #include <linux/bpf.h>
> #include <bpf/bpf_helpers.h>
>
> __attribute__((__noinline__))
> long skb_pull_data(struct __sk_buff *sk, __u32 len)
> {
> return bpf_skb_pull_data(sk, len);
> }
>
> SEC("tc")
> int test_invalidate_checks(struct __sk_buff *sk)
> {
> int *p = (void *)(long)sk->data;
> if ((void *)(p + 1) > (void *)(long)sk->data_end) return TCX_DROP;
> skb_pull_data(sk, 0);
> *p = 42;
> return TCX_PASS;
> }
>
> If I remove noinline or add static, the program is rejected as expected.
>
Hi Nick,
Thank you for the report. This is a bug. Technically, packet pointers
are invalidated by clear_all_pkt_pointers() called from check_helper_callf().
This functions looks through all packets in current verifier state.
However, global functions are verified independent of call sites,
so pointer 'p' does not exist in verifier state when 'skb_pull_data'
is verified, and thus is not invalidated.
