Re: Proposal: bpf_copy_from_user_remote

[Gabriele <phoenix1987@gmail.com>]

Hi Kenny

Your patch series looks neat! I haven't come across access_process_vm
during my source exploration. Indeed, passing a process descriptor
seems like a good idea. I presume one would then use, e.g.,
find_task_by_pid_ns to convert a pid+ns to a struct task_struct.

My use cases are about general observability into runtimes like
Python. For profiling, I would like to make a BPF version of Austin.
There is a variant for Linux that can be used to collect native
(including kernel) stacks (see
https://github.com/P403n1x87/austin#native-frame-stack), but this
works in a "traditional" way, using ptrace via libunwind. My idea is
to implement Python stack unwinding as a BPF program so that native
and runtime stacks could be both collected and then interleaved, like
austinp currently does. I think that, perhaps, I'd need a sleepable
version of the perf_event section to achieve this.

I have debugging use cases for Python in mind too, in particular for
native extensions. I believe these are similar to your C++ use cases.
I don't expect to be needing to iterate over all running tasks, so as
long as the new helper can be used against specific processes that can
be identified via pid (and namespace) then I'm totally fine with your
patch series.

Cheers,
Gab

On Thu, 13 Jan 2022 at 23:37, Kenny Yu <kennyyu@fb.com> wrote:
>
> Hi Gabriele,
>
> I just submitted a patch series that adds a similar helper to read
> userspace memory from a remote process, please see: https://lore.kernel.org/bpf/20220113233158.1582743-1-kennyyu@fb.com/T/#ma0646f96bccf0b957793054de7404115d321079d
>
> In my patch series, I added a bpf helper to wrap `access_process_vm`
> which takes a `struct task_struct` argument instead of a pid.
>
> In your patch series, one issue would be it is not clear which pid namespace
> the pid belongs to, whereas passing a `struct task_struct` is unambiguous.
> I think the helper signature in my patch series also provides more flexibility,
> as the bpf program can also provide different flags on how to read
> userspace memory.
>
> Our use case at Meta for this change is to use a bpf task iterator program
> to read debug information from a running process in production, e.g.,
> extract C++ async stack traces from a running program.
>
> A few questions:
> * What is your use case for adding this helper?
> * Do you have a specific requirement that requires using a pid, or would a
>   helper using `struct task_struct` be sufficient?
> * Are you ok with these changes? If so, I will proceed with my patch series.
>
> Thanks,
> Kenny Yu



-- 
"Egli è scritto in lingua matematica, e i caratteri son triangoli,
cerchi, ed altre figure
geometriche, senza i quali mezzi è impossibile a intenderne umanamente parola;
senza questi è un aggirarsi vanamente per un oscuro laberinto."

-- G. Galilei, Il saggiatore.