On Fri, Sep 27, 2019 at 9:59 AM Yonghong Song <yhs@xxxxxx> wrote:
>
>
>
> On 9/27/19 9:15 AM, Andrii Nakryiko wrote:
> > On Thu, Sep 26, 2019 at 1:15 AM Carlos Neira <cneirabustos@xxxxxxxxx> wrote:
> >>
> >> New bpf helper bpf_get_ns_current_pid_tgid,
> >> This helper will return pid and tgid from current task
> >> which namespace matches dev_t and inode number provided,
> >> this will allows us to instrument a process inside a container.
> >>
> >> Signed-off-by: Carlos Neira <cneirabustos@xxxxxxxxx>
> >> ---
> >> include/linux/bpf.h | 1 +
> >> include/uapi/linux/bpf.h | 18 +++++++++++++++++-
> >> kernel/bpf/core.c | 1 +
> >> kernel/bpf/helpers.c | 32 ++++++++++++++++++++++++++++++++
> >> kernel/trace/bpf_trace.c | 2 ++
> >> 5 files changed, 53 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> >> index 5b9d22338606..231001475504 100644
> >> --- a/include/linux/bpf.h
> >> +++ b/include/linux/bpf.h
> >> @@ -1055,6 +1055,7 @@ extern const struct bpf_func_proto bpf_get_local_storage_proto;
> >> extern const struct bpf_func_proto bpf_strtol_proto;
> >> extern const struct bpf_func_proto bpf_strtoul_proto;
> >> extern const struct bpf_func_proto bpf_tcp_sock_proto;
> >> +extern const struct bpf_func_proto bpf_get_ns_current_pid_tgid_proto;
> >>
> >> /* Shared helpers among cBPF and eBPF. */
> >> void bpf_user_rnd_init_once(void);
> >> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> >> index 77c6be96d676..9272dc8fb08c 100644
> >> --- a/include/uapi/linux/bpf.h
> >> +++ b/include/uapi/linux/bpf.h
> >> @@ -2750,6 +2750,21 @@ union bpf_attr {
> >> * **-EOPNOTSUPP** kernel configuration does not enable SYN cookies
> >> *
> >> * **-EPROTONOSUPPORT** IP packet version is not 4 or 6
> >> + *
> >> + * int bpf_get_ns_current_pid_tgid(u32 dev, u64 inum)
> >> + * Return
> >> + * A 64-bit integer containing the current tgid and pid from current task
> >
> > Function signature doesn't correspond to the actual return type (int vs u64).
> >
> >> + * which namespace inode and dev_t matches , and is create as such:
> >> + * *current_task*\ **->tgid << 32 \|**
> >> + * *current_task*\ **->pid**.
> >> + *
> >> + * On failure, the returned value is one of the following:
> >> + *
> >> + * **-EINVAL** if dev and inum supplied don't match dev_t and inode number
> >> + * with nsfs of current task.
> >> + *
> >> + * **-ENOENT** if /proc/self/ns does not exists.
> >> + *
> >> */
> >
> > [...]
> >
> >> #include "../../lib/kstrtox.h"
> >>
> >> @@ -487,3 +489,33 @@ const struct bpf_func_proto bpf_strtoul_proto = {
> >> .arg4_type = ARG_PTR_TO_LONG,
> >> };
> >> #endif
> >> +
> >> +BPF_CALL_2(bpf_get_ns_current_pid_tgid, u32, dev, u64, inum)
> >
> > Just curious, is dev_t officially specified as u32 and is never
> > supposed to grow bigger? I wonder if accepting u64 might be more
> > future-proof API here?
>
> This is what we have now in kernel (include/linux/types.h)
> typedef u32 __kernel_dev_t;
> typedef __kernel_dev_t dev_t;
>
> But userspace dev_t (defined at /usr/include/sys/types.h) have
> 8 bytes.
>
> Agree. Let us just use u64. It won't hurt and also will be fine
> if kernel internal dev_t becomes 64bit.
Sounds good. Let's not forget to check that conversion to dev_t
doesn't loose high bits, something like:
if ((u64)(dev_t)dev != dev)
return -E<something>;
>
> >
> >> +{
> >> + struct task_struct *task = current;
> >> + struct pid_namespace *pidns;
> >
> > [...]
> >
