From: Eduard Zingerman <eddyz87@gmail.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
bpf@vger.kernel.org, daniel@iogearbox.net,
martin.lau@kernel.org, kernel-team@meta.com
Subject: Re: [PATCH v3 bpf-next 03/10] bpf: fix check for attempt to corrupt spilled pointer
Date: Tue, 05 Dec 2023 15:34:43 +0200 [thread overview]
Message-ID: <7aa8af01db4fdcbedab376423d3960c22016aba3.camel@gmail.com> (raw)
In-Reply-To: <CAEf4Bzb8LouFSSX5DED_ucgq_xuhukE1BQ7y=hxY0c17Nq4T+Q@mail.gmail.com>
On Mon, 2023-12-04 at 19:56 -0800, Andrii Nakryiko wrote:
[...]
> > > So it makes me feel like the intent was to reject any partial writes
> > > with spilled reg slots. We could probably improve that to just make
> > > sure that we don't turn spilled pointers into STACK_MISC in unpriv,
> > > but I'm not sure if it's worth doing that instead of keeping things
> > > simple?
> >
> > You mean like below?
> >
> > if (!env->allow_ptr_leaks &&
> > is_spilled_reg(&state->stack[spi]) &&
> > is_spillable_regtype(state->stack[spi].spilled_ptr.type) &&
>
> Honestly, I wouldn't trust is_spillable_regtype() the way it's
> written, it's too easy to forget to add a new register type to the
> list. I think the only "safe to spill" register is probably
> SCALAR_VALUE, so I'd just do `type != SCALAR_VALUE`.
>
> But yes, I think that's the right approach.
'type != SCALAR_VALUE' makes sense as well.
Do you plan to add this check as a part of current patch?
> If we were being pedantic, though, we'd need to take into account
> offset and see if [offset, offset + size) overlaps with any
> STACK_SPILL/STACK_DYNPTR/STACK_ITER slots.
>
> But tbh, given it's unpriv programs we are talking about, I probably
> wouldn't bother extending this logic too much.
Yes, that's definitely is an ommission.
next prev parent reply other threads:[~2023-12-05 13:34 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-04 19:25 [PATCH v3 bpf-next 00/10] Complete BPF verifier precision tracking support for register spills Andrii Nakryiko
2023-12-04 19:25 ` [PATCH v3 bpf-next 01/10] bpf: support non-r10 register spill/fill to/from stack in precision tracking Andrii Nakryiko
2023-12-04 19:25 ` [PATCH v3 bpf-next 02/10] selftests/bpf: add stack access precision test Andrii Nakryiko
2023-12-04 19:25 ` [PATCH v3 bpf-next 03/10] bpf: fix check for attempt to corrupt spilled pointer Andrii Nakryiko
2023-12-04 22:12 ` Eduard Zingerman
2023-12-04 22:15 ` Eduard Zingerman
2023-12-05 0:23 ` Andrii Nakryiko
2023-12-05 0:54 ` Eduard Zingerman
2023-12-05 3:56 ` Andrii Nakryiko
2023-12-05 13:34 ` Eduard Zingerman [this message]
2023-12-05 18:30 ` Andrii Nakryiko
2023-12-05 18:49 ` Eduard Zingerman
2023-12-05 18:55 ` Andrii Nakryiko
2023-12-05 1:45 ` Alexei Starovoitov
2023-12-05 3:50 ` Andrii Nakryiko
2023-12-04 19:25 ` [PATCH v3 bpf-next 04/10] bpf: preserve STACK_ZERO slots on partial reg spills Andrii Nakryiko
2023-12-04 19:25 ` [PATCH v3 bpf-next 05/10] selftests/bpf: validate STACK_ZERO is preserved on subreg spill Andrii Nakryiko
2023-12-04 19:25 ` [PATCH v3 bpf-next 06/10] bpf: preserve constant zero when doing partial register restore Andrii Nakryiko
2023-12-04 19:25 ` [PATCH v3 bpf-next 07/10] selftests/bpf: validate zero preservation for sub-slot loads Andrii Nakryiko
2023-12-04 19:25 ` [PATCH v3 bpf-next 08/10] bpf: track aligned STACK_ZERO cases as imprecise spilled registers Andrii Nakryiko
2023-12-04 19:26 ` [PATCH v3 bpf-next 09/10] selftests/bpf: validate precision logic in partial_stack_load_preserves_zeros Andrii Nakryiko
2023-12-04 19:26 ` [PATCH v3 bpf-next 10/10] bpf: use common instruction history across all states Andrii Nakryiko
2023-12-04 22:32 ` [PATCH v3 bpf-next 00/10] Complete BPF verifier precision tracking support for register spills Andrii Nakryiko
2023-12-04 23:02 ` Yonghong Song
2023-12-04 23:52 ` Andrii Nakryiko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7aa8af01db4fdcbedab376423d3960c22016aba3.camel@gmail.com \
--to=eddyz87@gmail.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@meta.com \
--cc=martin.lau@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.