Re: [RFC PATCH bpf-next 1/5] bpf: fix potential error return

[Jiri Olsa <olsajiri@gmail.com>]

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@isovalent.com>

nice catch

Acked-by: Jiri Olsa <jolsa@kernel.org>

> ---
>  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'

jirka

>  }
>  
>  static void bpf_prog_kallsyms_del_subprogs(struct bpf_prog *fp)
> -- 
> 2.34.1
>