| Message ID | 20200304204157.58695-1-cneirabustos@gmail.com |
|---|---|
| Headers | show |
| Series | BPF: New helper to obtain namespace data from current task | expand |
On Wed, Mar 4, 2020 at 12:42 PM Carlos Neira <cneirabustos@gmail.com> wrote: > > 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. > > In the future different pid_ns files may belong to different devices, according to the > discussion between Eric Biederman and Yonghong in 2017 Linux plumbers conference. > To address that situation the helper requires inum and dev_t from /proc/self/ns/pid. > 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. Applied. Thanks. There was one spurious trailing whitespace that I fixed in patch 3 and missing .gitignore update for test_current_pid_tgid_new_ns. Could you please follow up with another patch to fold test_current_pid_tgid_new_ns into test_progs. I'd really like to consolidate all tests into single binary.
2020-03-12 17:45 UTC-0700 ~ Alexei Starovoitov <alexei.starovoitov@gmail.com> > On Wed, Mar 4, 2020 at 12:42 PM Carlos Neira <cneirabustos@gmail.com> wrote: >> >> 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. >> >> In the future different pid_ns files may belong to different devices, according to the >> discussion between Eric Biederman and Yonghong in 2017 Linux plumbers conference. >> To address that situation the helper requires inum and dev_t from /proc/self/ns/pid. >> 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. > > Applied. Thanks. > There was one spurious trailing whitespace that I fixed in patch 3 > and missing .gitignore update for test_current_pid_tgid_new_ns. > Could you please follow up with another patch to fold > test_current_pid_tgid_new_ns into test_progs. > I'd really like to consolidate all tests into single binary. > Compiling bpftool (with libbpf now relying on bpf_helper_defs.h generated from helpers documentation), I observe the following warning: .output/bpf_helper_defs.h:2834:72: warning: declaration of 'struct bpf_pidns_info' will not be visible outside of this function [-Wvisibility] static int (*bpf_get_ns_current_pid_tgid)(__u64 dev, __u64 ino, struct bpf_pidns_info *nsdata, __u32 size) = (void *) 120; Would it be possible to address this as part of the follow-up too, please? I think the fix would be to add "struct bpf_pidns_info" to type_fds (I see it was added to known_types already) in scripts/bpf_helpers_doc.py. Thanks, Quentin
On Thu, Mar 12, 2020 at 05:45:09PM -0700, Alexei Starovoitov wrote: > On Wed, Mar 4, 2020 at 12:42 PM Carlos Neira <cneirabustos@gmail.com> wrote: > > > > 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. > > > > In the future different pid_ns files may belong to different devices, according to the > > discussion between Eric Biederman and Yonghong in 2017 Linux plumbers conference. > > To address that situation the helper requires inum and dev_t from /proc/self/ns/pid. > > 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. > > Applied. Thanks. > There was one spurious trailing whitespace that I fixed in patch 3 > and missing .gitignore update for test_current_pid_tgid_new_ns. > Could you please follow up with another patch to fold > test_current_pid_tgid_new_ns into test_progs. > I'd really like to consolidate all tests into single binary. Thank you very much Alexei, I'll start working on the follow up patch to add test_current_pid_tgid_new_ns into test_progs. Bests
On Fri, Mar 13, 2020 at 10:39:41AM +0000, Quentin Monnet wrote: > 2020-03-12 17:45 UTC-0700 ~ Alexei Starovoitov <alexei.starovoitov@gmail.com> > > On Wed, Mar 4, 2020 at 12:42 PM Carlos Neira <cneirabustos@gmail.com> wrote: > >> > >> 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. > >> > >> In the future different pid_ns files may belong to different devices, according to the > >> discussion between Eric Biederman and Yonghong in 2017 Linux plumbers conference. > >> To address that situation the helper requires inum and dev_t from /proc/self/ns/pid. > >> 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. > > > > Applied. Thanks. > > There was one spurious trailing whitespace that I fixed in patch 3 > > and missing .gitignore update for test_current_pid_tgid_new_ns. > > Could you please follow up with another patch to fold > > test_current_pid_tgid_new_ns into test_progs. > > I'd really like to consolidate all tests into single binary. > > > > Compiling bpftool (with libbpf now relying on bpf_helper_defs.h > generated from helpers documentation), I observe the following > warning: > > .output/bpf_helper_defs.h:2834:72: warning: declaration of 'struct bpf_pidns_info' will not be visible outside of this function [-Wvisibility] > static int (*bpf_get_ns_current_pid_tgid)(__u64 dev, __u64 ino, struct bpf_pidns_info *nsdata, __u32 size) = (void *) 120; > > Would it be possible to address this as part of the follow-up too, > please? I think the fix would be to add "struct bpf_pidns_info" > to type_fds (I see it was added to known_types already) in > scripts/bpf_helpers_doc.py. > > Thanks, > Quentin Thanks for checking this out Quentin, I'm sorry I'll start working on this follow-up patch to fix this. Bests
On Fri, Mar 13, 2020 at 5:48 AM Carlos Antonio Neira Bustos <cneirabustos@gmail.com> wrote: > > On Thu, Mar 12, 2020 at 05:45:09PM -0700, Alexei Starovoitov wrote: > > On Wed, Mar 4, 2020 at 12:42 PM Carlos Neira <cneirabustos@gmail.com> wrote: > > > > > > 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. > > > > > > In the future different pid_ns files may belong to different devices, according to the > > > discussion between Eric Biederman and Yonghong in 2017 Linux plumbers conference. > > > To address that situation the helper requires inum and dev_t from /proc/self/ns/pid. > > > 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. > > > > Applied. Thanks. > > There was one spurious trailing whitespace that I fixed in patch 3 > > and missing .gitignore update for test_current_pid_tgid_new_ns. > > Could you please follow up with another patch to fold > > test_current_pid_tgid_new_ns into test_progs. > > I'd really like to consolidate all tests into single binary. > > Thank you very much Alexei, > I'll start working on the follow up patch to add test_current_pid_tgid_new_ns into test_progs. > Hey Carlos, Do you still plan to fold test_current_pid_tgid_new_ns into test_progs? > Bests
On Mon, Apr 27, 2020 at 6:44 PM carlos antonio neira bustos <cneirabustos@gmail.com> wrote: > > Hi, > > I’m sorry I’ll do the work needed this week. > Thanks for the heads up. > > Bests. No worries, thanks! > > > El El lun, 27 de abr. de 2020 a la(s) 21:40, Andrii Nakryiko <andrii.nakryiko@gmail.com> escribió: >> >> On Fri, Mar 13, 2020 at 5:48 AM Carlos Antonio Neira Bustos >> <cneirabustos@gmail.com> wrote: >> > >> > On Thu, Mar 12, 2020 at 05:45:09PM -0700, Alexei Starovoitov wrote: >> > > On Wed, Mar 4, 2020 at 12:42 PM Carlos Neira <cneirabustos@gmail.com> wrote: >> > > > >> > > > 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. >> > > > >> > > > In the future different pid_ns files may belong to different devices, according to the >> > > > discussion between Eric Biederman and Yonghong in 2017 Linux plumbers conference. >> > > > To address that situation the helper requires inum and dev_t from /proc/self/ns/pid. >> > > > 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. >> > > >> > > Applied. Thanks. >> > > There was one spurious trailing whitespace that I fixed in patch 3 >> > > and missing .gitignore update for test_current_pid_tgid_new_ns. >> > > Could you please follow up with another patch to fold >> > > test_current_pid_tgid_new_ns into test_progs. >> > > I'd really like to consolidate all tests into single binary. >> > >> > Thank you very much Alexei, >> > I'll start working on the follow up patch to add test_current_pid_tgid_new_ns into test_progs. >> > >> >> Hey Carlos, >> >> Do you still plan to fold test_current_pid_tgid_new_ns into test_progs? >> >> > Bests
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. In the future different pid_ns files may belong to different devices, according to the discussion between Eric Biederman and Yonghong in 2017 Linux plumbers conference. To address that situation the helper requires inum and dev_t from /proc/self/ns/pid. 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. Signed-off-by: Carlos Neira <cneirabustos@gmail.com> Carlos Neira (3): fs/nsfs.c: added ns_match bpf: added new helper bpf_get_ns_current_pid_tgid tools/testing/selftests/bpf: Add self-tests for new helper bpf_get_ns_current_pid_tgid. fs/nsfs.c | 14 ++ include/linux/bpf.h | 1 + include/linux/proc_ns.h | 2 + include/uapi/linux/bpf.h | 20 ++- kernel/bpf/core.c | 1 + kernel/bpf/helpers.c | 45 +++++ kernel/trace/bpf_trace.c | 2 + scripts/bpf_helpers_doc.py | 1 + tools/include/uapi/linux/bpf.h | 20 ++- tools/testing/selftests/bpf/Makefile | 3 +- .../bpf/prog_tests/ns_current_pid_tgid.c | 88 ++++++++++ .../bpf/progs/test_ns_current_pid_tgid.c | 37 ++++ .../bpf/test_current_pid_tgid_new_ns.c | 159 ++++++++++++++++++ 13 files changed, 390 insertions(+), 3 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c create mode 100644 tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c create mode 100644 tools/testing/selftests/bpf/test_current_pid_tgid_new_ns.c