Re: bpf_probe_read_user EFAULT

[Jiri Olsa <olsajiri@gmail.com>]

On Mon, Jan 02, 2023 at 11:07:50PM +0100, Victor Laforet wrote:
> Thanks!
> 
> I have tried to use bpf_copy_from_user_task() in place of bpf_probe_read_user() however I cannot seem to run my program. It fails with 'unknown func bpf_copy_from_user_task’.
> If I understood correctly, this function should be in ‘bpf/bpf_helpers.h’?

the declaration is in bpf_helper_defs.h, which is included by
bpf_helpers.h, so you need to #include it

> 
> Another quick question:
> I have set the bpf program as sleepable using ‘	bpf_program__set_flags(skel, BPF_F_SLEEPABLE);'
> I couldn’t find any other way to do that. Is it the right way to set it sleepable?

should work, but you could specify that directly in the program
section name, like SEC("fentry.s/...")

and it's just certain program types that can sleep:

	[jolsa@krava bpf]$ grep SEC_SLEEPABLE libbpf.c
	...
        SEC_DEF("uprobe.s+",            KPROBE, 0, SEC_SLEEPABLE, attach_uprobe),
        SEC_DEF("uretprobe.s+",         KPROBE, 0, SEC_SLEEPABLE, attach_uprobe),
        SEC_DEF("fentry.s+",            TRACING, BPF_TRACE_FENTRY, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_trace),
        SEC_DEF("fmod_ret.s+",          TRACING, BPF_MODIFY_RETURN, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_trace),
        SEC_DEF("fexit.s+",             TRACING, BPF_TRACE_FEXIT, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_trace),
        SEC_DEF("lsm.s+",               LSM, BPF_LSM_MAC, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_lsm),
        SEC_DEF("iter.s+",              TRACING, BPF_TRACE_ITER, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_iter),
        SEC_DEF("syscall",              SYSCALL, 0, SEC_SLEEPABLE),

jirka


> 
> Victor
> On 28 Dec 2022 at 20:41 +0100, Yonghong Song <yhs@meta.com>, wrote:
> >
> >
> > On 12/28/22 6:00 AM, Victor Laforet wrote:
> > > Yes I am sorry I did not mention that the example I sent was a minimal working example. I am filtering the events to select only preempted and events with the right pid as prev.
> > >
> > > Would bpf_copy_from_user_task work better in this setting than bpf_probe_read_user ?
> > > I don’t really understand why bpf_probe_read_user would not work for this use case.
> >
> > Right, bpf_copy_from_user_task() is better than bpf_probe_read_user().
> > You could also use bpf_copy_from_user() if you have target_pid checking.
> >
> > It is possible that the user variable you intended to access is not in
> > memory. In such cases, bpf_probe_read_user() will return EFAULT. But
> > bpf_copy_from_user() and bpf_copy_from_user_task() will go through
> > page fault process to bring the variable to the memory.
> > Also because of this extra work, bpf_copy_from_user() and
> > bpf_copy_from_user_task() only work for sleepable programs.
> >
> > >
> > > Victor
> > >
> > > ----- Mail original -----
> > > De: "Jiri Olsa" <olsajiri@gmail.com>
> > > À: "Victor Laforet" <victor.laforet@ip-paris.fr>
> > > Cc: "bpf" <bpf@vger.kernel.org>
> > > Envoyé: Mardi 27 Décembre 2022 17:00:42
> > > Objet: Re: bpf_probe_read_user EFAULT
> > >
> > > On Tue, Dec 27, 2022 at 03:56:06PM +0100, Victor Laforet wrote:
> > > > Hi all,
> > > >
> > > > I am trying to use bpf_probe_read_user to read a user space value from BPF. The issue is that I am getting -14 (-EFAULT) result from bpf_probe_read_user. I haven’t been able to make this function work reliably. Sometimes I get no error code then it goes back to EFAULT.
> > > >
> > > > I am seeking your help to try and make this code work.
> > > > Thank you!
> > > >
> > > > My goal is to read the variable pid on every bpf event.
> > > > Here is a full example:
> > > > (cat /sys/kernel/debug/tracing/trace_pipe to read the output).
> > > >
> > > > sched_switch.bpf.c
> > > > ```
> > > > #include "vmlinux.h"
> > > > #include <bpf/bpf_helpers.h>
> > > >
> > > > int *input_pid;
> > > >
> > > > char _license[4] SEC("license") = "GPL";
> > > >
> > > > SEC("tp_btf/sched_switch")
> > > > int handle_sched_switch(u64 *ctx)
> > >
> > > you might want to filter for your task, because sched_switch
> > > tracepoint is called for any task scheduler switch
> > >
> > > check BPF_PROG macro in bpf selftests on how to access tp_btf
> > > arguments from context, for sched_switch it's:
> > >
> > > TP_PROTO(bool preempt,
> > > struct task_struct *prev,
> > > struct task_struct *next,
> > > unsigned int prev_state),
> > >
> > > and call the read helper only for prev->pid == 'your app pid',
> > >
> > > there's bpf_copy_from_user_task helper you could use to read
> > > another task's user memory reliably, but it needs to be called
> > > from sleepable probe and you need to have the task pointer
> > >
> > > jirka
> > >
> > > > {
> > > > int pid;
> > > > int err;
> > > >
> > > > err = bpf_probe_read_user(&pid, sizeof(int), (void *)input_pid);
> > > > if (err != 0)
> > > > {
> > > > bpf_printk("Error on bpf_probe_read_user(pid) -> %d.\n", err);
> > > > return 0;
> > > > }
> > > >
> > > > bpf_printk("pid %d.\n", pid);
> > > > return 0;
> > > > }
> > > > ```
> > > >
> > > > sched_switch.c
> > > > ```
> > > > #include <stdio.h>
> > > > #include <unistd.h>
> > > > #include <sys/resource.h>
> > > > #include <bpf/libbpf.h>
> > > > #include "sched_switch.skel.h"
> > > > #include <time.h>
> > > >
> > > > static int libbpf_print_fn(enum libbpf_print_level level, const char *format, va_list args)
> > > > {
> > > > return vfprintf(stderr, format, args);
> > > > }
> > > >
> > > > int main(int argc, char **argv)
> > > > {
> > > > struct sched_switch_bpf *skel;
> > > > int err;
> > > > int pid = getpid();
> > > >
> > > > libbpf_set_print(libbpf_print_fn);
> > > >
> > > > skel = sched_switch_bpf__open();
> > > > if (!skel)
> > > > {
> > > > fprintf(stderr, "Failed to open BPF skeleton\n");
> > > > return 1;
> > > > }
> > > >
> > > > skel->bss->input_pid = &pid;
> > > >
> > > > err = sched_switch_bpf__load(skel);
> > > > if (err)
> > > > {
> > > > fprintf(stderr, "Failed to load and verify BPF skeleton\n");
> > > > goto cleanup;
> > > > }
> > > >
> > > > err = sched_switch_bpf__attach(skel);
> > > > if (err)
> > > > {
> > > > fprintf(stderr, "Failed to attach BPF skeleton\n");
> > > > goto cleanup;
> > > > }
> > > >
> > > > while (1);
> > > >
> > > > cleanup:
> > > > sched_switch_bpf__destroy(skel);
> > > > return -err;
> > > > }
> > > > ```