On Tue, Jan 30, 2024 at 12:20:06PM +0100, Jiri Olsa wrote:
> On Mon, Jan 22, 2024 at 04:49:32PM +0000, Anton Protopopov 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@xxxxxxxxxxxxx>
>
> nice catch
>
> Acked-by: Jiri Olsa <jolsa@xxxxxxxxxx>
>
> > ---
> > kernel/bpf/core.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > index fbb1d95a9b44..9ba9e0ea9c45 100644
> > --- a/kernel/bpf/core.c
> > +++ b/kernel/bpf/core.c
> > @@ -532,6 +532,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
> > */
> > @@ -539,7 +541,12 @@ 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);
> > + if (err)
> > + return err;
> > +
> > + return 0;
>
> could be just 'return err'
Thanks. I am inserting some code in a consequent patch in between,
so left this in this form
> jirka
>
> > }
> >
> > static void bpf_prog_kallsyms_del_subprogs(struct bpf_prog *fp)
> > --
> > 2.34.1
> >
