On Wed, Mar 20, 2019 at 06:23:20PM -0700, Alexei Starovoitov wrote:
> On Wed, Mar 20, 2019 at 01:49:22PM -0300, Carlos Antonio Neira Bustos wrote:
> >
> > This is a series of patches to introduce a new helper called
> > bpf_get_current_pidns_info,
> > this change has been splitted into the following patches:
> >
> > 1- Feature introduction
> > 2- Update tools/.../bpf.h
> > 3- Self tests and samples
> >
> >
> > From 852a65906122b05b4d1a23af868b2c245d240402 Mon Sep 17 00:00:00 2001
> > From: Carlos <cneirabus...@gmail.com>
> > Date: Tue, 19 Mar 2019 19:38:48 -0300
> > Subject: [PATCH] [PATCH bpf-next 1/3] BPF: New helper to obtain namespace
> > data
> > from current task
> >
> > This helper obtains the active namespace from current and returns pid, tgid,
> > device and namespace id as seen from that namespace, allowing to instrument
> > a process inside a container.
> > Device is read from /proc/self/ns/pid, as in the future it's possible that
> > different pid_ns files may belong to different devices, according
> > to the discussion between Eric Biederman and Yonghong in 2017 linux plumbers
> > conference.
> > Currently bpf_get_current_pid_tgid(), is used to do pid filtering in bcc's
> > scripts but this helper returns the pid as seen by the root namespace which
> > is
> > fine when a bcc script is not executed inside a container.
> > When the process of interest is inside a container, pid filtering will not
> > work
> > if bpf_get_current_pid_tgid() is used. This helper addresses this limitation
> > returning the pid as it's seen by the current namespace where the script is
> > executing.
> >
> > This helper has the same use cases as bpf_get_current_pid_tgid() as it can
> > be
> > used to do pid filtering even inside a container.
> >
> > For example a bcc script using bpf_get_current_pid_tgid()
> > (tools/funccount.py):
> >
> > u32 pid = bpf_get_current_pid_tgid() >> 32;
> > if (pid != <pid_arg_passed_in>)
> > return 0;
> > Could be modified to use bpf_get_current_pidns_info() as follows:
> >
> > struct bpf_pidns pidns;
> > bpf_get_current_pidns_info(&pidns, sizeof(struct bpf_pidns));
> > u32 pid = pidns.tgid;
> > u32 nsid = pidns.nsid;
> > if ((pid != <pid_arg_passed_in>) && (nsid != <nsid_arg_passed_in>))
> > return 0;
> >
> > To find out the name PID namespace id of a process, you could use this
> > command:
> >
> > $ ps -h -o pidns -p <pid_of_interest>
> >
> > Or this other command:
> >
> > $ ls -Li /proc/<pid_of_interest>/ns/pid
> >
> > Signed-off-by: Carlos Antonio Neira Bustos <cneirabus...@gmail.com>
> > -
> > ---
> > include/linux/bpf.h | 1 +
> > include/uapi/linux/bpf.h | 26 ++++++++++++++++++-
> > kernel/bpf/core.c | 1 +
> > kernel/bpf/helpers.c | 67
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> > kernel/trace/bpf_trace.c | 2 ++
> > 5 files changed, 96 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index a2132e09dc1c..a77f5bd77bd8 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -930,6 +930,7 @@ extern const struct bpf_func_proto
> > bpf_sk_redirect_map_proto;
> > extern const struct bpf_func_proto bpf_spin_lock_proto;
> > extern const struct bpf_func_proto bpf_spin_unlock_proto;
> > extern const struct bpf_func_proto bpf_get_local_storage_proto;
> > +extern const struct bpf_func_proto bpf_get_current_pidns_info_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 3c38ac9a92a7..facc701c7873 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -2366,6 +2366,18 @@ union bpf_attr {
> > * current value is ect (ECN capable). Works with IPv6 and
> > IPv4.
> > * Return
> > * 1 if set, 0 if not set.
> > + *
> > + * int bpf_get_current_pidns_info(struct bpf_pidns_info *pidns, u32
> > size_of_pidns)
> > + * Description
> > + * Copies into *pidns* pid, namespace id and tgid as seen by the
> > + * current namespace and also device from /proc/self/ns/pid.
> > + * *size_of_pidns* must be the size of *pidns*
> > + *
> > + * This helper is used when pid filtering is needed inside a
> > + * container as bpf_get_current_tgid() helper returns always the
> > + * pid id as seen by the root namespace.
> > + * Return
> > + * 0 on success -EINVAL on error.
> > */
> > #define __BPF_FUNC_MAPPER(FN) \
> > FN(unspec), \
> > @@ -2465,7 +2477,8 @@ union bpf_attr {
> > FN(spin_unlock), \
> > FN(sk_fullsock), \
> > FN(tcp_sock), \
> > - FN(skb_ecn_set_ce),
> > + FN(skb_ecn_set_ce), \
> > + FN(get_current_pidns_info),
> >
> > /* integer value in 'imm' field of BPF_CALL instruction selects which
> > helper
> > * function eBPF program intends to call
> > @@ -3152,4 +3165,15 @@ struct bpf_line_info {
> > struct bpf_spin_lock {
> > __u32 val;
> > };
> > +
> > +/* helper bpf_get_current_pidns_info will store the following
> > + * data, dev will contain major/minor from /proc/self/pid.
> > +*/
> > +struct bpf_pidns_info {
> > + __u32 dev;
> > + __u32 nsid;
> > + __u32 tgid;
> > + __u32 pid;
> > +};
> > +
> > #endif /* _UAPI__LINUX_BPF_H__ */
> > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > index 3f08c257858e..06329fbed95f 100644
> > --- a/kernel/bpf/core.c
> > +++ b/kernel/bpf/core.c
> > @@ -2044,6 +2044,7 @@ const struct bpf_func_proto
> > bpf_get_current_uid_gid_proto __weak;
> > const struct bpf_func_proto bpf_get_current_comm_proto __weak;
> > const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
> > const struct bpf_func_proto bpf_get_local_storage_proto __weak;
> > +const struct bpf_func_proto bpf_get_current_pidns_info __weak;
> >
> > const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
> > {
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index a411fc17d265..95c3780a6ba7 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -18,6 +18,11 @@
> > #include <linux/sched.h>
> > #include <linux/uidgid.h>
> > #include <linux/filter.h>
> > +#include <linux/pid_namespace.h>
> > +#include <linux/major.h>
> > +#include <linux/stat.h>
> > +#include <linux/namei.h>
> > +#include <linux/version.h>
> >
> > /* If kernel subsystem is allowing eBPF programs to call this function,
> > * inside its own verifier_ops->get_func_proto() callback it should return
> > @@ -364,3 +369,65 @@ const struct bpf_func_proto
> > bpf_get_local_storage_proto = {
> > };
> > #endif
> > #endif
> > +
> > +BPF_CALL_2(bpf_get_current_pidns_info, struct bpf_pidns_info *,
> > pidns_info, u32,
> > + size)
> > +{
> > + const char *nspid = "/proc/self/ns/pid";
> > + struct pid_namespace *pidns = NULL;
> > + struct kstat ks;
> > + struct path kp;
> > + pid_t tgid = 0;
> > + pid_t pid = 0;
> > + int res = 0;
> > +
> > + if (unlikely(size != sizeof(struct bpf_pidns_info)))
> > + goto clear;
> > +
> > + pidns = task_active_pid_ns(current);
> > +
> > + if (unlikely(!pidns))
> > + goto clear;
> > +
> > + pidns_info->nsid = pidns->ns.inum;
> > + pid = task_pid_nr_ns(current, pidns);
> > +
> > + if (unlikely(!pid))
> > + goto clear;
> > +
> > + tgid = task_tgid_nr_ns(current, pidns);
> > +
> > + if (unlikely(!tgid))
> > + goto clear;
> > +
> > + pidns_info->tgid = (u32) tgid;
> > + pidns_info->pid = (u32) pid;
> > +
> > + kern_path(nspid, 0, &kp);
> > +
> > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,11,0)
> > + res = vfs_getattr(&kp, &ks, STATX_ALL, 0);
> > +#else
> > + res = vfs_getattr(&kp, &ks);
> > +#endif
>
> Please access namespaces directly.
> I suspect bpf helpers cannot do vfs_getattr. Something inside might sleep
> while bpf progs are non preemptible.
>
> Also please run ./scripts/checkpatch.pl on your patches
> and read Documentation/bpf/bpf_devel_QA.rst.
>
Hello Alexei,
Thanks for checking this out and also thanks for the documentation link.
At this point I have a couple of questions:
* What is the reason to not use the current namespace api instead of directly
accessing namespaces?.
* Regarding bpf programs not being preemptible. If we add spin_locks to the
vfs_getattr call, would that be an acceptable solution?
spin_lock(&bpf_lock);
res = vfs_getattr(&kp, &ks);
spin_unlock(&bpf_lock);
Is there another way to interact with the vfs layer within a bpf helper?.
Bests
