| Message ID | 20191001214141.6294-3-cneirabustos@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | BPF: New helper to obtain namespace data from current task | expand |
On 10/1/19 11:41 PM, Carlos Neira 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@gmail.com> > --- > include/linux/bpf.h | 1 + > include/uapi/linux/bpf.h | 18 +++++++++++++++++- > kernel/bpf/core.c | 1 + > kernel/bpf/helpers.c | 36 ++++++++++++++++++++++++++++++++++++ > kernel/trace/bpf_trace.c | 2 ++ > 5 files changed, 57 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..ea8145d7f897 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 > + * > + * u64 bpf_get_ns_current_pid_tgid(u64 dev, u64 inum) > + * Return > + * A 64-bit integer containing the current tgid and pid from current task > + * 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, or if dev conversion to dev_t lost high bits. > + * > + * **-ENOENT** if /proc/self/ns does not exists. > + * > */ > #define __BPF_FUNC_MAPPER(FN) \ > FN(unspec), \ > @@ -2862,7 +2877,8 @@ union bpf_attr { > FN(sk_storage_get), \ > FN(sk_storage_delete), \ > FN(send_signal), \ > - FN(tcp_gen_syncookie), > + FN(tcp_gen_syncookie), \ > + FN(get_ns_current_pid_tgid), > > /* integer value in 'imm' field of BPF_CALL instruction selects which helper > * function eBPF program intends to call > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c > index 66088a9e9b9e..b2fd5358f472 100644 > --- a/kernel/bpf/core.c > +++ b/kernel/bpf/core.c > @@ -2042,6 +2042,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_ns_current_pid_tgid_proto __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 5e28718928ca..8777181d1717 100644 > --- a/kernel/bpf/helpers.c > +++ b/kernel/bpf/helpers.c > @@ -11,6 +11,8 @@ > #include <linux/uidgid.h> > #include <linux/filter.h> > #include <linux/ctype.h> > +#include <linux/pid_namespace.h> > +#include <linux/proc_ns.h> > > #include "../../lib/kstrtox.h" > > @@ -487,3 +489,37 @@ const struct bpf_func_proto bpf_strtoul_proto = { > .arg4_type = ARG_PTR_TO_LONG, > }; > #endif > + > +BPF_CALL_2(bpf_get_ns_current_pid_tgid, u64, dev, u64, inum) > +{ > + struct task_struct *task = current; > + struct pid_namespace *pidns; > + pid_t pid, tgid; > + > + if ((u64)(dev_t)dev != dev) > + return -EINVAL; > + > + if (unlikely(!task)) > + return -EINVAL; > + > + pidns = task_active_pid_ns(task); > + if (unlikely(!pidns)) > + return -ENOENT; > + > + > + if (!ns_match(&pidns->ns, (dev_t)dev, inum)) > + return -EINVAL; > + > + pid = task_pid_nr_ns(task, pidns); > + tgid = task_tgid_nr_ns(task, pidns); > + > + return (u64) tgid << 32 | pid; Basically here you are overlapping the 64-bit return value for the valid outcome with the error codes above for the invalid case. If you look at bpf_perf_event_read() we already had such broken occasion that bit us in the past, and needed to introduce bpf_perf_event_read_value() instead. Lets not go there again and design it similarly to the latter. > +} > + > +const struct bpf_func_proto bpf_get_ns_current_pid_tgid_proto = { > + .func = bpf_get_ns_current_pid_tgid, > + .gpl_only = false, > + .ret_type = RET_INTEGER, > + .arg1_type = ARG_ANYTHING, > + .arg2_type = ARG_ANYTHING, > +}; > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 44bd08f2443b..32331a1dcb6d 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -735,6 +735,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) > #endif > case BPF_FUNC_send_signal: > return &bpf_send_signal_proto; > + case BPF_FUNC_get_ns_current_pid_tgid: > + return &bpf_get_ns_current_pid_tgid_proto; > default: > return NULL; > } >
On Wed, Oct 02, 2019 at 12:52:29PM +0200, Daniel Borkmann wrote: > On 10/1/19 11:41 PM, Carlos Neira 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@gmail.com> > > --- > > include/linux/bpf.h | 1 + > > include/uapi/linux/bpf.h | 18 +++++++++++++++++- > > kernel/bpf/core.c | 1 + > > kernel/bpf/helpers.c | 36 ++++++++++++++++++++++++++++++++++++ > > kernel/trace/bpf_trace.c | 2 ++ > > 5 files changed, 57 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..ea8145d7f897 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 > > + * > > + * u64 bpf_get_ns_current_pid_tgid(u64 dev, u64 inum) > > + * Return > > + * A 64-bit integer containing the current tgid and pid from current task > > + * 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, or if dev conversion to dev_t lost high bits. > > + * > > + * **-ENOENT** if /proc/self/ns does not exists. > > + * > > */ > > #define __BPF_FUNC_MAPPER(FN) \ > > FN(unspec), \ > > @@ -2862,7 +2877,8 @@ union bpf_attr { > > FN(sk_storage_get), \ > > FN(sk_storage_delete), \ > > FN(send_signal), \ > > - FN(tcp_gen_syncookie), > > + FN(tcp_gen_syncookie), \ > > + FN(get_ns_current_pid_tgid), > > /* integer value in 'imm' field of BPF_CALL instruction selects which helper > > * function eBPF program intends to call > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c > > index 66088a9e9b9e..b2fd5358f472 100644 > > --- a/kernel/bpf/core.c > > +++ b/kernel/bpf/core.c > > @@ -2042,6 +2042,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_ns_current_pid_tgid_proto __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 5e28718928ca..8777181d1717 100644 > > --- a/kernel/bpf/helpers.c > > +++ b/kernel/bpf/helpers.c > > @@ -11,6 +11,8 @@ > > #include <linux/uidgid.h> > > #include <linux/filter.h> > > #include <linux/ctype.h> > > +#include <linux/pid_namespace.h> > > +#include <linux/proc_ns.h> > > #include "../../lib/kstrtox.h" > > @@ -487,3 +489,37 @@ const struct bpf_func_proto bpf_strtoul_proto = { > > .arg4_type = ARG_PTR_TO_LONG, > > }; > > #endif > > + > > +BPF_CALL_2(bpf_get_ns_current_pid_tgid, u64, dev, u64, inum) > > +{ > > + struct task_struct *task = current; > > + struct pid_namespace *pidns; > > + pid_t pid, tgid; > > + > > + if ((u64)(dev_t)dev != dev) > > + return -EINVAL; > > + > > + if (unlikely(!task)) > > + return -EINVAL; > > + > > + pidns = task_active_pid_ns(task); > > + if (unlikely(!pidns)) > > + return -ENOENT; > > + > > + > > + if (!ns_match(&pidns->ns, (dev_t)dev, inum)) > > + return -EINVAL; > > + > > + pid = task_pid_nr_ns(task, pidns); > > + tgid = task_tgid_nr_ns(task, pidns); > > + > > + return (u64) tgid << 32 | pid; > > Basically here you are overlapping the 64-bit return value for the valid > outcome with the error codes above for the invalid case. If you look at > bpf_perf_event_read() we already had such broken occasion that bit us in > the past, and needed to introduce bpf_perf_event_read_value() instead. > Lets not go there again and design it similarly to the latter. > > > +} > > + > > +const struct bpf_func_proto bpf_get_ns_current_pid_tgid_proto = { > > + .func = bpf_get_ns_current_pid_tgid, > > + .gpl_only = false, > > + .ret_type = RET_INTEGER, > > + .arg1_type = ARG_ANYTHING, > > + .arg2_type = ARG_ANYTHING, > > +}; > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > index 44bd08f2443b..32331a1dcb6d 100644 > > --- a/kernel/trace/bpf_trace.c > > +++ b/kernel/trace/bpf_trace.c > > @@ -735,6 +735,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) > > #endif > > case BPF_FUNC_send_signal: > > return &bpf_send_signal_proto; > > + case BPF_FUNC_get_ns_current_pid_tgid: > > + return &bpf_get_ns_current_pid_tgid_proto; > > default: > > return NULL; > > } > > > Daniel, If I understand correctly, to avoid problems I need to change the helper's function signature to something like the following: struct bpf_ns_current_pid_tgid_storage { __u64 dev; __u64 inum; __u64 pidtgid; }; BPF_CALL_2(bpf_get_ns_current_pid_tgid, struct bpf_ns_current_pid_tgid_storage *, buf, u32, size); then use dev and inum provided by the user and return the requested value into pidtgid of the struct. Would that work? Thanks for your help.
On Thu, Oct 3, 2019 at 8:01 AM Carlos Antonio Neira Bustos <cneirabustos@gmail.com> wrote: > > On Wed, Oct 02, 2019 at 12:52:29PM +0200, Daniel Borkmann wrote: > > On 10/1/19 11:41 PM, Carlos Neira 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@gmail.com> > > > --- > > > include/linux/bpf.h | 1 + > > > include/uapi/linux/bpf.h | 18 +++++++++++++++++- > > > kernel/bpf/core.c | 1 + > > > kernel/bpf/helpers.c | 36 ++++++++++++++++++++++++++++++++++++ > > > kernel/trace/bpf_trace.c | 2 ++ > > > 5 files changed, 57 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..ea8145d7f897 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 > > > + * > > > + * u64 bpf_get_ns_current_pid_tgid(u64 dev, u64 inum) > > > + * Return > > > + * A 64-bit integer containing the current tgid and pid from current task > > > + * 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, or if dev conversion to dev_t lost high bits. > > > + * > > > + * **-ENOENT** if /proc/self/ns does not exists. > > > + * > > > */ > > > #define __BPF_FUNC_MAPPER(FN) \ > > > FN(unspec), \ > > > @@ -2862,7 +2877,8 @@ union bpf_attr { > > > FN(sk_storage_get), \ > > > FN(sk_storage_delete), \ > > > FN(send_signal), \ > > > - FN(tcp_gen_syncookie), > > > + FN(tcp_gen_syncookie), \ > > > + FN(get_ns_current_pid_tgid), > > > /* integer value in 'imm' field of BPF_CALL instruction selects which helper > > > * function eBPF program intends to call > > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c > > > index 66088a9e9b9e..b2fd5358f472 100644 > > > --- a/kernel/bpf/core.c > > > +++ b/kernel/bpf/core.c > > > @@ -2042,6 +2042,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_ns_current_pid_tgid_proto __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 5e28718928ca..8777181d1717 100644 > > > --- a/kernel/bpf/helpers.c > > > +++ b/kernel/bpf/helpers.c > > > @@ -11,6 +11,8 @@ > > > #include <linux/uidgid.h> > > > #include <linux/filter.h> > > > #include <linux/ctype.h> > > > +#include <linux/pid_namespace.h> > > > +#include <linux/proc_ns.h> > > > #include "../../lib/kstrtox.h" > > > @@ -487,3 +489,37 @@ const struct bpf_func_proto bpf_strtoul_proto = { > > > .arg4_type = ARG_PTR_TO_LONG, > > > }; > > > #endif > > > + > > > +BPF_CALL_2(bpf_get_ns_current_pid_tgid, u64, dev, u64, inum) > > > +{ > > > + struct task_struct *task = current; > > > + struct pid_namespace *pidns; > > > + pid_t pid, tgid; > > > + > > > + if ((u64)(dev_t)dev != dev) > > > + return -EINVAL; > > > + > > > + if (unlikely(!task)) > > > + return -EINVAL; > > > + > > > + pidns = task_active_pid_ns(task); > > > + if (unlikely(!pidns)) > > > + return -ENOENT; > > > + > > > + > > > + if (!ns_match(&pidns->ns, (dev_t)dev, inum)) > > > + return -EINVAL; > > > + > > > + pid = task_pid_nr_ns(task, pidns); > > > + tgid = task_tgid_nr_ns(task, pidns); > > > + > > > + return (u64) tgid << 32 | pid; > > > > Basically here you are overlapping the 64-bit return value for the valid > > outcome with the error codes above for the invalid case. If you look at > > bpf_perf_event_read() we already had such broken occasion that bit us in > > the past, and needed to introduce bpf_perf_event_read_value() instead. > > Lets not go there again and design it similarly to the latter. > > > > > +} > > > + > > > +const struct bpf_func_proto bpf_get_ns_current_pid_tgid_proto = { > > > + .func = bpf_get_ns_current_pid_tgid, > > > + .gpl_only = false, > > > + .ret_type = RET_INTEGER, > > > + .arg1_type = ARG_ANYTHING, > > > + .arg2_type = ARG_ANYTHING, > > > +}; > > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > > index 44bd08f2443b..32331a1dcb6d 100644 > > > --- a/kernel/trace/bpf_trace.c > > > +++ b/kernel/trace/bpf_trace.c > > > @@ -735,6 +735,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) > > > #endif > > > case BPF_FUNC_send_signal: > > > return &bpf_send_signal_proto; > > > + case BPF_FUNC_get_ns_current_pid_tgid: > > > + return &bpf_get_ns_current_pid_tgid_proto; > > > default: > > > return NULL; > > > } > > > > > > Daniel, > If I understand correctly, to avoid problems I need to change the helper's function signature to something like the following: > > struct bpf_ns_current_pid_tgid_storage { > __u64 dev; > __u64 inum; > __u64 pidtgid; if you do it this way, please do __u32 pid; __u34 tgid; I have to look up where pid and tgid is in that 64-bit number every single time, let's not do it here for no good reason. > }; > > BPF_CALL_2(bpf_get_ns_current_pid_tgid, > struct bpf_ns_current_pid_tgid_storage *, buf, u32, size); > > then use dev and inum provided by the user and return the requested > value into pidtgid of the struct. Would that work? > > Thanks for your help. > > > >
On Thu, Oct 03, 2019 at 10:30:06AM -0700, Andrii Nakryiko wrote: > On Thu, Oct 3, 2019 at 8:01 AM Carlos Antonio Neira Bustos > <cneirabustos@gmail.com> wrote: > > > > On Wed, Oct 02, 2019 at 12:52:29PM +0200, Daniel Borkmann wrote: > > > On 10/1/19 11:41 PM, Carlos Neira 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@gmail.com> > > > > --- > > > > include/linux/bpf.h | 1 + > > > > include/uapi/linux/bpf.h | 18 +++++++++++++++++- > > > > kernel/bpf/core.c | 1 + > > > > kernel/bpf/helpers.c | 36 ++++++++++++++++++++++++++++++++++++ > > > > kernel/trace/bpf_trace.c | 2 ++ > > > > 5 files changed, 57 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..ea8145d7f897 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 > > > > + * > > > > + * u64 bpf_get_ns_current_pid_tgid(u64 dev, u64 inum) > > > > + * Return > > > > + * A 64-bit integer containing the current tgid and pid from current task > > > > + * 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, or if dev conversion to dev_t lost high bits. > > > > + * > > > > + * **-ENOENT** if /proc/self/ns does not exists. > > > > + * > > > > */ > > > > #define __BPF_FUNC_MAPPER(FN) \ > > > > FN(unspec), \ > > > > @@ -2862,7 +2877,8 @@ union bpf_attr { > > > > FN(sk_storage_get), \ > > > > FN(sk_storage_delete), \ > > > > FN(send_signal), \ > > > > - FN(tcp_gen_syncookie), > > > > + FN(tcp_gen_syncookie), \ > > > > + FN(get_ns_current_pid_tgid), > > > > /* integer value in 'imm' field of BPF_CALL instruction selects which helper > > > > * function eBPF program intends to call > > > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c > > > > index 66088a9e9b9e..b2fd5358f472 100644 > > > > --- a/kernel/bpf/core.c > > > > +++ b/kernel/bpf/core.c > > > > @@ -2042,6 +2042,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_ns_current_pid_tgid_proto __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 5e28718928ca..8777181d1717 100644 > > > > --- a/kernel/bpf/helpers.c > > > > +++ b/kernel/bpf/helpers.c > > > > @@ -11,6 +11,8 @@ > > > > #include <linux/uidgid.h> > > > > #include <linux/filter.h> > > > > #include <linux/ctype.h> > > > > +#include <linux/pid_namespace.h> > > > > +#include <linux/proc_ns.h> > > > > #include "../../lib/kstrtox.h" > > > > @@ -487,3 +489,37 @@ const struct bpf_func_proto bpf_strtoul_proto = { > > > > .arg4_type = ARG_PTR_TO_LONG, > > > > }; > > > > #endif > > > > + > > > > +BPF_CALL_2(bpf_get_ns_current_pid_tgid, u64, dev, u64, inum) > > > > +{ > > > > + struct task_struct *task = current; > > > > + struct pid_namespace *pidns; > > > > + pid_t pid, tgid; > > > > + > > > > + if ((u64)(dev_t)dev != dev) > > > > + return -EINVAL; > > > > + > > > > + if (unlikely(!task)) > > > > + return -EINVAL; > > > > + > > > > + pidns = task_active_pid_ns(task); > > > > + if (unlikely(!pidns)) > > > > + return -ENOENT; > > > > + > > > > + > > > > + if (!ns_match(&pidns->ns, (dev_t)dev, inum)) > > > > + return -EINVAL; > > > > + > > > > + pid = task_pid_nr_ns(task, pidns); > > > > + tgid = task_tgid_nr_ns(task, pidns); > > > > + > > > > + return (u64) tgid << 32 | pid; > > > > > > Basically here you are overlapping the 64-bit return value for the valid > > > outcome with the error codes above for the invalid case. If you look at > > > bpf_perf_event_read() we already had such broken occasion that bit us in > > > the past, and needed to introduce bpf_perf_event_read_value() instead. > > > Lets not go there again and design it similarly to the latter. > > > > > > > +} > > > > + > > > > +const struct bpf_func_proto bpf_get_ns_current_pid_tgid_proto = { > > > > + .func = bpf_get_ns_current_pid_tgid, > > > > + .gpl_only = false, > > > > + .ret_type = RET_INTEGER, > > > > + .arg1_type = ARG_ANYTHING, > > > > + .arg2_type = ARG_ANYTHING, > > > > +}; > > > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > > > index 44bd08f2443b..32331a1dcb6d 100644 > > > > --- a/kernel/trace/bpf_trace.c > > > > +++ b/kernel/trace/bpf_trace.c > > > > @@ -735,6 +735,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) > > > > #endif > > > > case BPF_FUNC_send_signal: > > > > return &bpf_send_signal_proto; > > > > + case BPF_FUNC_get_ns_current_pid_tgid: > > > > + return &bpf_get_ns_current_pid_tgid_proto; > > > > default: > > > > return NULL; > > > > } > > > > > > > > > Daniel, > > If I understand correctly, to avoid problems I need to change the helper's function signature to something like the following: > > > > struct bpf_ns_current_pid_tgid_storage { > > __u64 dev; > > __u64 inum; > > __u64 pidtgid; > > if you do it this way, please do > > __u32 pid; > __u34 tgid; > > I have to look up where pid and tgid is in that 64-bit number every > single time, let's not do it here for no good reason. > > > }; > > > > BPF_CALL_2(bpf_get_ns_current_pid_tgid, > > struct bpf_ns_current_pid_tgid_storage *, buf, u32, size); > > > > then use dev and inum provided by the user and return the requested > > value into pidtgid of the struct. Would that work? > > > > Thanks for your help. > > > > > > > > Andrii, You are right, I'll do so. Bests.
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..ea8145d7f897 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 + * + * u64 bpf_get_ns_current_pid_tgid(u64 dev, u64 inum) + * Return + * A 64-bit integer containing the current tgid and pid from current task + * 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, or if dev conversion to dev_t lost high bits. + * + * **-ENOENT** if /proc/self/ns does not exists. + * */ #define __BPF_FUNC_MAPPER(FN) \ FN(unspec), \ @@ -2862,7 +2877,8 @@ union bpf_attr { FN(sk_storage_get), \ FN(sk_storage_delete), \ FN(send_signal), \ - FN(tcp_gen_syncookie), + FN(tcp_gen_syncookie), \ + FN(get_ns_current_pid_tgid), /* integer value in 'imm' field of BPF_CALL instruction selects which helper * function eBPF program intends to call diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 66088a9e9b9e..b2fd5358f472 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2042,6 +2042,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_ns_current_pid_tgid_proto __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 5e28718928ca..8777181d1717 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -11,6 +11,8 @@ #include <linux/uidgid.h> #include <linux/filter.h> #include <linux/ctype.h> +#include <linux/pid_namespace.h> +#include <linux/proc_ns.h> #include "../../lib/kstrtox.h" @@ -487,3 +489,37 @@ const struct bpf_func_proto bpf_strtoul_proto = { .arg4_type = ARG_PTR_TO_LONG, }; #endif + +BPF_CALL_2(bpf_get_ns_current_pid_tgid, u64, dev, u64, inum) +{ + struct task_struct *task = current; + struct pid_namespace *pidns; + pid_t pid, tgid; + + if ((u64)(dev_t)dev != dev) + return -EINVAL; + + if (unlikely(!task)) + return -EINVAL; + + pidns = task_active_pid_ns(task); + if (unlikely(!pidns)) + return -ENOENT; + + + if (!ns_match(&pidns->ns, (dev_t)dev, inum)) + return -EINVAL; + + pid = task_pid_nr_ns(task, pidns); + tgid = task_tgid_nr_ns(task, pidns); + + return (u64) tgid << 32 | pid; +} + +const struct bpf_func_proto bpf_get_ns_current_pid_tgid_proto = { + .func = bpf_get_ns_current_pid_tgid, + .gpl_only = false, + .ret_type = RET_INTEGER, + .arg1_type = ARG_ANYTHING, + .arg2_type = ARG_ANYTHING, +}; diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 44bd08f2443b..32331a1dcb6d 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -735,6 +735,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) #endif case BPF_FUNC_send_signal: return &bpf_send_signal_proto; + case BPF_FUNC_get_ns_current_pid_tgid: + return &bpf_get_ns_current_pid_tgid_proto; default: return NULL; }
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@gmail.com> --- include/linux/bpf.h | 1 + include/uapi/linux/bpf.h | 18 +++++++++++++++++- kernel/bpf/core.c | 1 + kernel/bpf/helpers.c | 36 ++++++++++++++++++++++++++++++++++++ kernel/trace/bpf_trace.c | 2 ++ 5 files changed, 57 insertions(+), 1 deletion(-)