diff mbox series

[v3,bpf-next,07/10] bpf: enforce precision of R0 on program/async callback return

Message ID 20231130000406.480870-8-andrii@kernel.org (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series BPF verifier retval logic fixes | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR fail merge-conflict
netdev/tree_selection success Clearly marked for bpf-next
netdev/apply fail Patch does not apply to bpf-next

Commit Message

Andrii Nakryiko Nov. 30, 2023, 12:04 a.m. UTC
Given we enforce a valid range for program and async callback return
value, we must mark R0 as precise to avoid incorrect state pruning.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 kernel/bpf/verifier.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Shung-Hsi Yu Nov. 30, 2023, 6:33 a.m. UTC | #1
On Wed, Nov 29, 2023 at 04:04:03PM -0800, Andrii Nakryiko wrote:
> Given we enforce a valid range for program and async callback return
> value, we must mark R0 as precise to avoid incorrect state pruning.

Looking at previous discussion[1], this commit fixes the potential
"out-of-range r0 got state pruned" issue. To my best knowledge that
means this commit would be needed all the way back in

Fixes: b5dc0163d8fd ("bpf: precise scalar_value tracking")

Is this wildly off?

1: https://lore.kernel.org/r/20231031050324.1107444-4-andrii@kernel.org

> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  kernel/bpf/verifier.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index c54944af1bcc..2cd150d6d141 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -15138,6 +15138,10 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
>  		return -EINVAL;
>  	}
>  
> +	err = mark_chain_precision(env, regno);
> +	if (err)
> +		return err;
> +
>  	if (!retval_range_within(range, reg)) {
>  		verbose_invalid_scalar(env, reg, range, exit_ctx, reg_name);
>  		if (!is_subprog &&
Andrii Nakryiko Nov. 30, 2023, 6:41 a.m. UTC | #2
On Wed, Nov 29, 2023 at 10:33 PM Shung-Hsi Yu <shung-hsi.yu@suse.com> wrote:
>
> On Wed, Nov 29, 2023 at 04:04:03PM -0800, Andrii Nakryiko wrote:
> > Given we enforce a valid range for program and async callback return
> > value, we must mark R0 as precise to avoid incorrect state pruning.
>
> Looking at previous discussion[1], this commit fixes the potential
> "out-of-range r0 got state pruned" issue. To my best knowledge that
> means this commit would be needed all the way back in
>
> Fixes: b5dc0163d8fd ("bpf: precise scalar_value tracking")
>
> Is this wildly off?

No, I think you are right. Added Fixes: tag as suggested.


>
> 1: https://lore.kernel.org/r/20231031050324.1107444-4-andrii@kernel.org
>
> > Acked-by: Eduard Zingerman <eddyz87@gmail.com>
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> >  kernel/bpf/verifier.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index c54944af1bcc..2cd150d6d141 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -15138,6 +15138,10 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
> >               return -EINVAL;
> >       }
> >
> > +     err = mark_chain_precision(env, regno);
> > +     if (err)
> > +             return err;
> > +
> >       if (!retval_range_within(range, reg)) {
> >               verbose_invalid_scalar(env, reg, range, exit_ctx, reg_name);
> >               if (!is_subprog &&
diff mbox series

Patch

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index c54944af1bcc..2cd150d6d141 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -15138,6 +15138,10 @@  static int check_return_code(struct bpf_verifier_env *env, int regno, const char
 		return -EINVAL;
 	}
 
+	err = mark_chain_precision(env, regno);
+	if (err)
+		return err;
+
 	if (!retval_range_within(range, reg)) {
 		verbose_invalid_scalar(env, reg, range, exit_ctx, reg_name);
 		if (!is_subprog &&