On Fri, Apr 21, 2023 at 8:52 AM Florian Westphal <fw@xxxxxxxxx> wrote:
>
> Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote:
> > On Thu, Apr 20, 2023 at 02:44:55PM +0200, Florian Westphal wrote:
> > > +
> > > +SEC("netfilter")
> > > +__description("netfilter valid context access")
> > > +__success __failure_unpriv
> > > +__retval(1)
> > > +__naked void with_invalid_ctx_access_test5(void)
> > > +{
> > > + asm volatile (" \
> > > + r2 = *(u64*)(r1 + %[__bpf_nf_ctx_state]); \
> > > + r1 = *(u64*)(r1 + %[__bpf_nf_ctx_skb]); \
> > > + r0 = 1; \
> > > + exit; \
> > > +" :
> > > + : __imm_const(__bpf_nf_ctx_state, offsetof(struct bpf_nf_ctx, state)),
> > > + __imm_const(__bpf_nf_ctx_skb, offsetof(struct bpf_nf_ctx, skb))
> > > + : __clobber_all);
> >
> > Could you write this one in C instead?
> >
> > Also check that skb and state are dereferenceable after that.
>
> My bad. Added this and that:
>
> SEC("netfilter")
> __description("netfilter valid context read and invalid write")
> __failure __msg("only read is supported")
> int with_invalid_ctx_access_test5(struct bpf_nf_ctx *ctx)
> {
> struct nf_hook_state *state = (void *)ctx->state;
>
> state->sk = NULL;
> return 1;
> }
>
> SEC("netfilter")
> __description("netfilter test prog with skb and state read access")
> __success __failure_unpriv
> __retval(0)
> int with_valid_ctx_access_test6(struct bpf_nf_ctx *ctx)
> {
> const struct nf_hook_state *state = ctx->state;
> struct sk_buff *skb = ctx->skb;
> const struct iphdr *iph;
> const struct tcphdr *th;
> u8 buffer_iph[20] = {};
> u8 buffer_th[40] = {};
> struct bpf_dynptr ptr;
> uint8_t ihl;
>
> if (skb->len <= 20 || bpf_dynptr_from_skb(skb, 0, &ptr))
> return 1;
Use NF_ACCEPT instead of 1 ?
Sadly it's not an enum yet, so it's not in vmlinux.h
The prog would need to manually #define it.
>
> iph = bpf_dynptr_slice(&ptr, 0, buffer_iph, sizeof(buffer_iph));
> if (!iph)
> return 1;
>
> if (state->pf != 2)
> return 1;
>
> ihl = iph->ihl << 2;
> th = bpf_dynptr_slice(&ptr, ihl, buffer_th, sizeof(buffer_th));
> if (!th)
> return 1;
>
> return th->dest == bpf_htons(22) ? 1 : 0;
> }
Perfect. That's what I wanted to see.
Without above example it's hard for people to see how ctx->skb
can be accessed to parse the packet.
> "Worksforme". Is there anything else thats missing?
> If not I'll send v5 on Monday.
ship it any time. Don't delay.
