Re: [PATCH 1/2] bpf: fix userspace access for bpf_probe_read{, str}()

["Zidenberg, Tsahi" <tsahee@amazon.com>]

On 30/03/2021 20:21, Sasha Levin wrote:
> commit 8d92db5c04d10381f4db70ed99b1b576f5db18a7 upstream.
>>
>> This is an adaptation of parts from above commit to kernel 5.4.
>
> This is very different from the upstream commit, let's not annotate it
> as that commit.
>
No problem.
>>
>> To generally fix the issue, upstream includes new BPF helpers:
>> bpf_probe_read_{user,kernel}_str(). For details on them, see
>> commit 6ae08ae3dea2 ("bpf: Add probe_read_{user, kernel} and probe_read_{user, kernel}_str helpers")
>
> What stops us from taking that API back to 5.4? I took a look at the
> dependencies and they don't look too scary.
>
> Can we try that route instead? We really don't want to diverge from
> upstream that much.
>
probe_read_{user,kernel}* functions themselves seem simple enough.
Importing them in a forward-compliant way to 5.4 would require either
adding an unused entry in bpf.h's __BPF_FUNC_MAPPER or also pulling
skb_output bpf helper functions into 5.4. To me, it seems like too
much of a UAPI change to go into a stable release.

Another option would be to import more code to make it somewhat closer
to upstream implementation without changing UAPI. As in v5.8, I could
internally map these helpers to probe_read_compat* functions, which
will in turn call probe_read_{user,kernel}*_common functions.
Func names might seem weird out of context, but it will be closer.
Implementation will still defer, e.g. to avoid warnings on platforms
without ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE

Does that sound like a reasonable solution?

--
Thanks,
Tsahi