On Wed, Nov 10, 2021 at 11:55 AM Hao Luo <haoluo@xxxxxxxxxx> wrote: > > On Tue, Nov 9, 2021 at 8:43 PM Andrii Nakryiko > <andrii.nakryiko@xxxxxxxxx> wrote: > > > > On Mon, Nov 8, 2021 at 4:31 PM Hao Luo <haoluo@xxxxxxxxxx> wrote: > > > > > > There are currently two ways to modify a kernel memory in bpf programs: > > > 1. declare a ksym of scalar type and directly modify its memory. > > > 2. Pass a RDONLY_BUF into a helper function which will override > > > its arguments. For example, bpf_d_path, bpf_snprintf. > > > > > > This patchset fixes these two problem. For the first, we introduce a > > > new reg type PTR_TO_RDONLY_MEM for the scalar typed ksym, which forbids > > > writing. For the second, we introduce a new arg type ARG_CONST_PTR_TO_MEM > > > to differentiate the arg types that only read the memory from those > > > that may write the memory. The previous ARG_PTR_TO_MEM is now only > > > compatible with writable memories. If a helper doesn't write into its > > > argument, it can use ARG_CONST_PTR_TO_MEM, which is also compatible > > > with read-only memories. > > > > > > In v2, Andrii suggested using the name "ARG_PTR_TO_RDONLY_MEM", but I > > > find it is sort of misleading. Because the new arg_type is compatible > > > with both write and read-only memory. So I chose ARG_CONST_PTR_TO_MEM > > > instead. > > > > I find ARG_CONST_PTR_TO_MEM misleading. It's the difference between > > `char * const` (const pointer to mutable memory) vs `const char *` > > (pointer to an immutable memory). We need the latter semantics, and > > that *is* PTR_TO_RDONLY_MEM in BPF verifier terms. > > > > Ah, I am aware of the semantic difference between 'char * const' and > 'const char *', but your explanation in the bracket helps me see your > point better. It does seem PTR_TO_RDONLY_MEM matches the semantics > now. Let me fix and send an update. I thought earlier we agreed that flag approach is prefered. Looks like OR_NULL discussion is progressing nicely and IS_RDONLY or IS_RDWR flags will just fit right in. What's the reason to go with this approach ? It seems it will make the refactoring more tedious later.
