On Thu, 28 Aug 2025 at 02:22, Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> wrote:
>
> On Wed, 27 Aug 2025 at 17:37, Puranjay Mohan <puranjay@xxxxxxxxxx> wrote:
> >
> > Begin reporting arena page faults and the faulting address to BPF
> > program's stderr, this patch adds support in the arm64 and x86-64 JITs,
> > support for other archs can be added later.
> >
> > The fault handlers receive the 32 bit address in the arena region so
> > the upper 32 bits of user_vm_start is added to it before printing the
> > address. This is what the user would expect to see as this is what is
> > printed by bpf_printk() is you pass it an address returned by
> > bpf_arena_alloc_pages();
> >
> > Signed-off-by: Puranjay Mohan <puranjay@xxxxxxxxxx>
> > Acked-by: Yonghong Song <yonghong.song@xxxxxxxxx>
> > ---
> > [...]
> >
> > bool ex_handler_bpf(const struct exception_table_entry *x, struct pt_regs *regs)
> > {
> > - u32 reg = x->fixup >> 8;
> > + u32 reg = FIELD_GET(FIXUP_REG_MASK, x->fixup);
> > + u32 insn_len = FIELD_GET(FIXUP_INSN_LEN_MASK, x->fixup);
> > + bool is_arena = !!(x->fixup & FIXUP_ARENA_ACCESS);
> > + bool is_write = (reg == DONT_CLEAR);
> > + unsigned long addr;
> > + s16 off;
> > + u32 arena_reg;
> >
> > /* jump over faulting load and clear dest register */
> > if (reg != DONT_CLEAR)
> > *(unsigned long *)((void *)regs + reg) = 0;
> > - regs->ip += x->fixup & 0xff;
> > + regs->ip += insn_len;
> > +
> > + if (is_arena) {
> > + arena_reg = FIELD_GET(FIXUP_ARENA_REG_MASK, x->fixup);
> > + off = FIELD_GET(DATA_ARENA_OFFSET_MASK, x->data);
> > + addr = *(unsigned long *)((void *)regs + arena_reg) + off;
>
> Same question. I faintly remember I spent a few hours when I
> implemented this, wondering why the reported address was always zeroed
> out for x86 before realizing they can be the same.
> It would be good to add a test for this condition.
> And also, to work around this, the address needs to be captured before
> the destination register is cleared.
To be clear, to have such a test, you'd want to write it in inline
assembly to make sure compiler shenanigans don't screw up things.
>
> > [...]
