From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Hao Luo <haoluo@google.com>, Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
KP Singh <kpsingh@kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next v2 2/3] bpf: Introduce ARG_PTR_TO_WRITABLE_MEM
Date: Tue, 26 Oct 2021 10:59:46 -0700 [thread overview]
Message-ID: <CAADnVQJQuZ9pP_T_ZDgoeTnqfPcRMcKM_BshBTpmsZiRmzWMgA@mail.gmail.com> (raw)
In-Reply-To: <CAEf4BzbvXQ1qpGazNKCBhzUUPmmfe9d9icDtf++weJkJmme0aw@mail.gmail.com>
On Mon, Oct 25, 2021 at 10:14 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
> >
> > Instead of adding new types,
> > can we do something like this instead:
> >
> > diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> > index c8a78e830fca..5dbd2541aa86 100644
> > --- a/include/linux/bpf_verifier.h
> > +++ b/include/linux/bpf_verifier.h
> > @@ -68,7 +68,8 @@ struct bpf_reg_state {
> > u32 btf_id;
> > };
> >
> > - u32 mem_size; /* for PTR_TO_MEM | PTR_TO_MEM_OR_NULL */
> > + u32 rd_mem_size; /* for PTR_TO_MEM | PTR_TO_MEM_OR_NULL */
> > + u32 wr_mem_size; /* for PTR_TO_MEM | PTR_TO_MEM_OR_NULL */
>
> This seems more confusing, it's technically possible to express a
> memory pointer from which you can read X bytes, but can write Y bytes.
I'm fine it being a new flag instead of wr_mem_size.
> I actually liked the idea that helpers will be explicit about whether
> they can write into a memory or only read from it.
>
> Apart from a few more lines of code, are there any downsides to having
> PTR_TO_MEM vs PTR_TO_RDONLY_MEM?
because it's a churn and non scalable long term.
It's not just PTR_TO_RDONLY_MEM.
It's also ARG_PTR_TO_RDONLY_MEM,
and RET_PTR_TO_RDONLY_MEM,
and PTR_TO_RDONLY_MEM_OR_NULL
and *_OR_BTF_ID,
and *_OR_BTF_ID_OR_NULL.
It felt that expressing readonly-ness as a flag in bpf_reg_state
will make it easier to get right in the code and extend in the future.
May be we will have a kernel vs user flag for PTR_TO_MEM in the future.
If we use different name to express that we will have:
PTR_TO_USER_RDONLY_MEM and
PTR_TO_USER_MEM
plus all variants of ARG_* and RET_* and *_OR_NULL.
With a flag approach it will be just another flag in bpf_reg_state.
next prev parent reply other threads:[~2021-10-26 17:59 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-25 23:12 [PATCH bpf-next v2 0/3] bpf: Prevent writing read-only memory Hao Luo
2021-10-25 23:12 ` [PATCH bpf-next 1/3] bpf: Prevent write to ksym memory Hao Luo
2021-10-25 23:12 ` [PATCH bpf-next v2 2/3] bpf: Introduce ARG_PTR_TO_WRITABLE_MEM Hao Luo
2021-10-26 3:48 ` Alexei Starovoitov
2021-10-26 5:14 ` Andrii Nakryiko
2021-10-26 17:59 ` Alexei Starovoitov [this message]
2021-10-26 18:13 ` Hao Luo
2021-10-26 18:44 ` Andrii Nakryiko
2021-10-26 19:22 ` Alexei Starovoitov
2021-10-26 21:24 ` Andrii Nakryiko
2021-10-26 5:06 ` Andrii Nakryiko
2021-10-26 17:51 ` Hao Luo
2021-10-26 18:53 ` Andrii Nakryiko
2021-10-26 20:00 ` Hao Luo
2021-10-25 23:12 ` [PATCH bpf-next v2 3/3] bpf/selftests: Test PTR_TO_RDONLY_MEM Hao Luo
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=CAADnVQJQuZ9pP_T_ZDgoeTnqfPcRMcKM_BshBTpmsZiRmzWMgA@mail.gmail.com \
--to=alexei.starovoitov@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=haoluo@google.com \
--cc=kpsingh@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).