Re: [PATCH bpf-next v2] bpf: Check return from set_memory_rox() and friends

[Daniel Borkmann]

On 3/15/24 3:55 PM, Christophe Leroy wrote:
[...]
> > >    {
> > >        WARN_ON_ONCE(size > PAGE_SIZE);
> > > -    set_memory_rox((long)image, 1);
> > > +    return set_memory_rox((long)image, 1);
> > >    }
> > > -void __weak arch_unprotect_bpf_trampoline(void *image, unsigned int
> > > size)
> > > +int __weak arch_unprotect_bpf_trampoline(void *image, unsigned int size)
> > >    {
> > > +    int err;
> > >        WARN_ON_ONCE(size > PAGE_SIZE);
> > > -    set_memory_nx((long)image, 1);
> > > -    set_memory_rw((long)image, 1);
> > > +
> > > +    err = set_memory_nx((long)image, 1);
> > > +    if (err)
> > > +        return err;
> > > +    return set_memory_rw((long)image, 1);
> > >    }
> >
> > Do we still need this? It looks like this does not have an in-tree user
> > anymore.
>
> Looks like last user went away with commit 187e2af05abe ("bpf:
> struct_ops supports more than one page for trampolines.") but I'm having
> hard time figuring if it's valid or not.
>
> But as there is no user anymore it surely can go away. Will you drop it
> or do you want a proper patch from me ?

My understanding is that the VM_FLUSH_RESET_PERMS would take care of this
via arch_alloc_bpf_trampoline(). Anyway, gvien there is a merge conflict
with this patch, pls include it with a v3.

Thanks,
Daniel