Re: [PATCH v4 bpf-next 6/7] bpf: fix potential error return

[Anton Protopopov <aspsk@isovalent.com>]

On 24/12/03 01:26PM, Andrii Nakryiko wrote:
> On Tue, Dec 3, 2024 at 5:49 AM Anton Protopopov <aspsk@isovalent.com> wrote:
> >
> > The bpf_remove_insns() function returns WARN_ON_ONCE(error), where
> > error is a result of bpf_adj_branches(), and thus should be always 0
> > However, if for any reason it is not 0, then it will be converted to
> > boolean by WARN_ON_ONCE and returned to user space as 1, not an actual
> > error value. Fix this by returning the original err after the WARN check.
> >
> > Signed-off-by: Anton Protopopov <aspsk@isovalent.com>
> > Acked-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  kernel/bpf/core.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> 
> Looks irrelevant to the patch set and probably should go through the
> bpf tree? I'll leave it up to Alexei to decide, though.

Sure, I can send it separately, if needed.

> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> 
> 
> > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > index a2327c4fdc8b..8b9711e6da6c 100644
> > --- a/kernel/bpf/core.c
> > +++ b/kernel/bpf/core.c
> > @@ -539,6 +539,8 @@ struct bpf_prog *bpf_patch_insn_single(struct bpf_prog *prog, u32 off,
> >
> >  int bpf_remove_insns(struct bpf_prog *prog, u32 off, u32 cnt)
> >  {
> > +       int err;
> > +
> >         /* Branch offsets can't overflow when program is shrinking, no need
> >          * to call bpf_adj_branches(..., true) here
> >          */
> > @@ -546,7 +548,9 @@ int bpf_remove_insns(struct bpf_prog *prog, u32 off, u32 cnt)
> >                 sizeof(struct bpf_insn) * (prog->len - off - cnt));
> >         prog->len -= cnt;
> >
> > -       return WARN_ON_ONCE(bpf_adj_branches(prog, off, off + cnt, off, false));
> > +       err = bpf_adj_branches(prog, off, off + cnt, off, false);
> > +       WARN_ON_ONCE(err);
> > +       return err;
> >  }
> >
> >  static void bpf_prog_kallsyms_del_subprogs(struct bpf_prog *fp)
> > --
> > 2.34.1
> >
> >