On Fri, Jan 20, 2023 at 12:09:34PM IST, Alexei Starovoitov wrote:
> On Fri, Jan 20, 2023 at 12:01:12PM +0530, Kumar Kartikeya Dwivedi wrote:
> > On Fri, Jan 20, 2023 at 11:50:41AM IST, Alexei Starovoitov wrote:
> > > On Fri, Jan 20, 2023 at 09:13:11AM +0530, Kumar Kartikeya Dwivedi wrote:
> > > > +
> > > > +SEC("?tc")
> > > > +__failure __msg("cannot overwrite referenced dynptr") __log_level(2)
> > > > +int dynptr_pruning_overwrite(struct __sk_buff *ctx)
> > > > +{
> > > > + asm volatile (
> > > > + "r9 = 0xeB9F;"
> > > > + "r6 = %[ringbuf] ll;"
> > > > + "r1 = r6;"
> > > > + "r2 = 8;"
> > > > + "r3 = 0;"
> > > > + "r4 = r10;"
> > > > + "r4 += -16;"
> > > > + "call %[bpf_ringbuf_reserve_dynptr];"
> > > > + "if r0 == 0 goto pjmp1;"
> > > > + "goto pjmp2;"
> > > > + "pjmp1:"
> > > > + "*(u64 *)(r10 - 16) = r9;"
> > > > + "pjmp2:"
> > > > + "r1 = r10;"
> > > > + "r1 += -16;"
> > > > + "r2 = 0;"
> > > > + "call %[bpf_ringbuf_discard_dynptr];"
> > >
> > > It should still work if we remove "" from every line, right?
> > > Would it be easier to read?
> >
> > You mean write it like this?
> >
> > asm volatile (
> > "r9 = 0xeB9F; \
> > r6 = %[ringbuf] ll; \
> > r1 = r6; \
> > r2 = 8; \
> > r3 = 0; \
> > r4 = r10; \
> > r4 += -16; \
> > call %[bpf_ringbuf_reserve_dynptr]; \
> > if r0 == 0 goto pjmp1; \
> > goto pjmp2; \
> > pjmp1: \
> > *(u64 *)(r10 - 16) = r9; \
> > pjmp2: \
> > r1 = r10; \
> > r1 += -16; \
> > r2 = 0; \
> > call %[bpf_ringbuf_discard_dynptr]; "
> > :
> > : __imm(bpf_ringbuf_reserve_dynptr),
> > __imm(bpf_ringbuf_discard_dynptr),
> > __imm_addr(ringbuf)
> > : __clobber_all
> > );
> >
> > I guess that does look a bit cleaner, if you think the same I can try converting
> > them.
>
> Only asking to consider different options because once we start adding tests
> in this form everyone will copy paste the style.
> In verifier/precise.c we use:
> .errstr =
> "26: (85) call bpf_probe_read_kernel#113\
> last_idx 26 first_idx 22\
> regs=4 stack=0 before 25\
> regs=4 stack=0 before 24\
> regs=4 stack=0 before 23\
> regs=4 stack=0 before 22\
>
> so the following is another option:
> asm volatile (
> "r9 = 0xeB9F;\
> r6 = %[ringbuf] ll;\
> r1 = r6;\
> r2 = 8;\
> r3 = 0;\
> r4 = r10;\
> r4 += -16;
>
> My vote goes to your 2nd approach where every \ is tab-aligned to the right.
Yeah, understood. I will convert to this style and respin. Thanks.
