perf: Container-aware tracing support
RFC PATCH 1/1 perf: Container-aware tracing support
Current tracing infrastructure such as perf and ftrace reports system wide data when invoked inside a container. It is required to restrict events specific to a container context when such tools are invoked inside a container.
This RFC patch supports filtering container specific events, without any change in the user interface, when invoked within a container for the perf utility; such support needs to be extended to ftrace. This patch assumes that the debugfs is available within the container and all the processes running inside a container are grouped into a single perf_event subsystem of cgroups. This patch piggybacks on the existing support available for tracing with cgroups [1] by setting the cgrp member of the event structure to the cgroup of the context perf tool is invoked from.
However, this patch is not complete and requires more work to fully support tracing inside a container. This patch is intended to initiate the discussion on having container-aware tracing support. A detailed explanation on what is supported and pending issues are mentioned below.
Suggestions, feedback, flames are welcome.
[1] https://lkml.org/lkml/2011/2/14/40
--------------------------------------------------------------------
Details:
With this patch, perf-stat, perf-record (tracepoints, [ku]rpobes) and perf-top when executed within a container reports events that are triggered only in that container context. However, there are couple of limitations on how this works for kprobes/uprobes and in general ftrace infrastructure.
The problem arises due to the use of files /sys/kernel/debug/ tracing/[uk]probe_events. Perf utility inserts a probe by writing into the [uk]probe_events file, which is parsed by the kernel to register an event. When debugfs is mounted inside containers, the contents of these files are visible to all containers. This implies that a user within a container can list/delete probes registered by other containers, leading to security issues and/or denial of service (Eg:
by deleting a probe from another container every time it is registered). This could be undesirable depending on the way containers are used (Eg: if used in multi-tenancy with each users assigned a container).
The issues mentioned above exist for tracing infrastructures which use ftrace interface. One approach is to have a container specific view of these files under /sys/kernel/debug/tracing. At this moment, this seems to require a significant rework of ftrace.
We are looking for feedback on the assumptions we have made about the processes running inside a container grouped into a single perf_event subsystem and also any thoughts on extending such support to ftrace.
Regards,
Aravinda
--- kernel/events/core.c | 49 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 14 deletions(-)
kernel/events/core.c +35 -14
@@ -589,17 +589,38 @@ static inline int perf_cgroup_connect(int fd, struct perf_event *event, {struct perf_cgroup *cgrp;struct cgroup_subsys_state *css; -struct fd f =fdget (fd); +struct fd f;int ret =0 ; -if (!f.file) -return -EBADF ; +if (fd !=-1 ) { + f =fdget (fd); +if (!f.file) +return -EBADF ; - css =css_tryget_online_from_dir (f.file->f_path.dentry, + css =css_tryget_online_from_dir (f.file->f_path.dentry, &perf_event_cgrp_subsys); -if (IS_ERR (css)) { - ret =PTR_ERR (css); - goto out; +if (IS_ERR (css)) { + ret =PTR_ERR (css); +fdput (f); +return ret; + } + }else if (event->attach_state ==PERF_ATTACH_TASK ) { +/* Tracing on a PID. No need to set event->cgrp */ +return ret; + }else if (task_active_pid_ns (current) != &init_pid_ns) { +/* Don't set event->cgrp if task belongs to root cgroup */ +if (task_css_is_root (current, perf_event_cgrp_id)) +return ret; + + css =task_css (current, perf_event_cgrp_id); +if (!css || !css_tryget_online (css)) +return -ENOENT ; + }else { +/* +* perf invoked from global context and hence don't set +* event->cgrp as all the events should be included +*/ +return ret; } cgrp = container_of(css, struct perf_cgroup, css);
@@ -614,8 +635,10 @@ static inline int perf_cgroup_connect(int fd, struct perf_event *event,perf_detach_cgroup (event); ret = -EINVAL ; } -out: -fdput (f); + +if (fd !=-1 ) +fdput (f); +return ret; }
@@ -7554,11 +7577,9 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,if (!has_branch_stack (event)) event->attr.branch_sample_type =0 ; -if (cgroup_fd !=-1 ) { - err =perf_cgroup_connect (cgroup_fd, event, attr, group_leader); -if (err) - goto err_ns; - } + err =perf_cgroup_connect (cgroup_fd, event, attr, group_leader); +if (err) + goto err_ns; pmu =perf_init_event (event);if (!pmu)
Peter Zijlstra
maintainerQuoted 15 lines · Aravinda Prasad
Current tracing infrastructure such as perf and ftrace reports system wide data when invoked inside a container. It is required to restrict events specific to a container context when such tools are invoked inside a container.
This RFC patch supports filtering container specific events, without any change in the user interface, when invoked within a container for the perf utility; such support needs to be extended to ftrace. This patch assumes that the debugfs is available within the container and all the processes running inside a container are grouped into a single
perf_eventsubsystem of cgroups. This patch piggybacks on the existing support available for tracing with cgroups [1] by setting the cgrp member of the event structure to the cgroup of the context perf tool is invoked from.
Quoted 5 lines
However, this patch is not complete and requires more work to fully support tracing inside a container. This patch is intended to initiate the discussion on having container-aware tracing support. A detailed explanation on what is supported and pending issues are mentioned below.
tracing is outside the scope of perf; I suspect you want tracefs to be sensitive to filesystem namespaces and all that that entails.
Quoted 30 lines
Cc Hari Bathini <[email protected]>Signed-off-by Aravinda Prasad <[email protected]>--- kernel/events/core.c | 49 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 14 deletions(-)kernel/events/core.c +6
-6@@ -589,17 +589,38 @@ static inline int perf_cgroup_connect(int fd, struct perf_event *event, {struct perf_cgroup *cgrp;struct cgroup_subsys_state *css; -struct fd f =fdget (fd); +struct fd f;int ret =0 ; -if (!f.file) -return -EBADF ; +if (fd !=-1 ) { + f =fdget (fd); +if (!f.file) +return -EBADF ; - css =css_tryget_online_from_dir (f.file->f_path.dentry, + css =css_tryget_online_from_dir (f.file->f_path.dentry, &perf_event_cgrp_subsys); -if (IS_ERR (css)) { - ret =PTR_ERR (css);
kernel/events/core.c static inline int perf_cgroup_connect(int fd, struct perf_event *event, · line 610
Quoted 10 lines
kernel/events/core.c +9
-1- goto out; +if (IS_ERR (css)) { + ret =PTR_ERR (css); +fdput (f); +return ret; + } + }else if (event->attach_state ==PERF_ATTACH_TASK ) { +/* Tracing on a PID. No need to set event->cgrp */ +return ret; + }else if (task_active_pid_ns (current) != &init_pid_ns) {
Why the pid namespace?
kernel/events/core.c static inline int perf_cgroup_connect(int fd, struct perf_event *event, · line 613
Quoted 3 lines
kernel/events/core.c +3
-0+/* Don't set event->cgrp if task belongs to root cgroup */ +if (task_css_is_root (current, perf_event_cgrp_id)) +return ret;
So if you have the root perf_cgroup inside your container you can escape?
Quoted 13 lines
kernel/events/core.c +10
-0+ + css =task_css (current, perf_event_cgrp_id); +if (!css || !css_tryget_online (css)) +return -ENOENT ; + }else { +/* +* perf invoked from global context and hence don't set +* event->cgrp as all the events should be included +*/ +return ret; } cgrp = container_of(css, struct perf_cgroup, css);
Quoted 16 lines · Peter Zijlstra
Quoted 15 lines · Aravinda Prasad
Current tracing infrastructure such as perf and ftrace reports system wide data when invoked inside a container. It is required to restrict events specific to a container context when such tools are invoked inside a container.
This RFC patch supports filtering container specific events, without any change in the user interface, when invoked within a container for the perf utility; such support needs to be extended to ftrace. This patch assumes that the debugfs is available within the container and all the processes running inside a container are grouped into a single
perf_eventsubsystem of cgroups. This patch piggybacks on the existing support available for tracing with cgroups [1] by setting the cgrp member of the event structure to the cgroup of the context perf tool is invoked from.
Quoted 8 lines
Quoted 5 lines
However, this patch is not complete and requires more work to fully support tracing inside a container. This patch is intended to initiate the discussion on having container-aware tracing support. A detailed explanation on what is supported and pending issues are mentioned below.
tracing is outside the scope of perf; I suspect you want tracefs to be sensitive to filesystem namespaces and all that that entails.
Yes, tracefs needs to be sensitive to filesystem namespace. I wanted to put together points required for supporting perf/trace inside containers.
Quoted 35 lines
Quoted 35 lines
Cc Hari Bathini <[email protected]>Signed-off-by Aravinda Prasad <[email protected]>--- kernel/events/core.c | 49 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 14 deletions(-)kernel/events/core.c +9
-7@@ -589,17 +589,38 @@ static inline int perf_cgroup_connect(int fd, struct perf_event *event, {struct perf_cgroup *cgrp;struct cgroup_subsys_state *css; -struct fd f =fdget (fd); +struct fd f;int ret =0 ; -if (!f.file) -return -EBADF ; +if (fd !=-1 ) { + f =fdget (fd); +if (!f.file) +return -EBADF ; - css =css_tryget_online_from_dir (f.file->f_path.dentry, + css =css_tryget_online_from_dir (f.file->f_path.dentry, &perf_event_cgrp_subsys); -if (IS_ERR (css)) { - ret =PTR_ERR (css); - goto out; +if (IS_ERR (css)) { + ret =PTR_ERR (css); +fdput (f);
Quoted 8 lines
kernel/events/core.c static inline int perf_cgroup_connect(int fd, struct perf_event *event, · line 610
Quoted 6 lines
kernel/events/core.c +6
-0+return ret; + } + }else if (event->attach_state ==PERF_ATTACH_TASK ) { +/* Tracing on a PID. No need to set event->cgrp */ +return ret; + }else if (task_active_pid_ns (current) != &init_pid_ns) {Why the pid namespace?
This comes from my understanding of container -- having at least a separate PID namespace with processes inside a container grouped into a single perf_event cgroups subsystem.
I know there are other ways to define a container, however, I thought I start with the above one.
Quoted 7 lines
kernel/events/core.c static inline int perf_cgroup_connect(int fd, struct perf_event *event, · line 613
Quoted 4 lines
kernel/events/core.c +3
-0+/* Don't set event->cgrp if task belongs to root cgroup */ +if (task_css_is_root (current, perf_event_cgrp_id)) +return ret;So if you have the root
perf_cgroupinside your container you can escape?
If we have root perf_cgroup inside the container then even if we set event->cgrp we will be including all processes in the system.
Regards,
Aravinda
Quoted 15 lines
Quoted 14 lines
kernel/events/core.c +10
-0+ + css =task_css (current, perf_event_cgrp_id); +if (!css || !css_tryget_online (css)) +return -ENOENT ; + }else { +/* +* perf invoked from global context and hence don't set +* event->cgrp as all the events should be included +*/ +return ret; } cgrp = container_of(css, struct perf_cgroup, css);
Signature · 2 lines
Regards, Aravinda
Peter Zijlstra
maintainerQuoted 10 lines · Aravinda Prasad
Quoted 3 lines
kernel/events/core.c static inline int perf_cgroup_connect(int fd, struct perf_event *event, · line 610
Quoted 1 line
kernel/events/core.c +1
-0+ }else if (task_active_pid_ns (current) != &init_pid_ns) {Why the pid namespace?
This comes from my understanding of container -- having at least a separate PID namespace with processes inside a container grouped into a single
perf_eventcgroups subsystem.I know there are other ways to define a container, however, I thought I start with the above one.
Right, but you should at least mention this, preferably in a comment.
Quoted 10 lines
Quoted 7 lines
kernel/events/core.c static inline int perf_cgroup_connect(int fd, struct perf_event *event, · line 613
Quoted 4 lines
kernel/events/core.c +3
-0+/* Don't set event->cgrp if task belongs to root cgroup */ +if (task_css_is_root (current, perf_event_cgrp_id)) +return ret;So if you have the root
perf_cgroupinside your container you can escape?If we have root
perf_cgroupinside the container then even if we setevent->cgrpwe will be including all processes in the system.
Yes, that's what I said. Why does that make sense?
Quoted 5 lines · Peter Zijlstra
Quoted 4 lines · Aravinda Prasad
Quoted 3 lines
kernel/events/core.c static inline int perf_cgroup_connect(int fd, struct perf_event *event, · line 610
Quoted 1 line
kernel/events/core.c +1
-0+ }else if (task_active_pid_ns (current) != &init_pid_ns) {Why the pid namespace?
Quoted 8 lines
Quoted 6 lines
This comes from my understanding of container -- having at least a separate PID namespace with processes inside a container grouped into a single
perf_eventcgroups subsystem.I know there are other ways to define a container, however, I thought I start with the above one.
Right, but you should at least mention this, preferably in a comment.
Yes. I should have done that.
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
kernel/events/core.c static inline int perf_cgroup_connect(int fd, struct perf_event *event, · line 613
Quoted 5 lines
kernel/events/core.c +3
-0+/* Don't set event->cgrp if task belongs to root cgroup */ +if (task_css_is_root (current, perf_event_cgrp_id)) +return ret;So if you have the root
perf_cgroupinside your container you can escape?
Quoted 4 lines
Quoted 2 lines
If we have root
perf_cgroupinside the container then even if we setevent->cgrpwe will be including all processes in the system.Yes, that's what I said. Why does that make sense?
We assume that processes are grouped into a single perf_event subsystem. If we have root perf_cgroup, from our assumption, implies we are not invoked from a container context. However, not sure if this assumption is right.
Quoted 1 line
Signature · 2 lines
Regards, Aravinda
Quoted 16 lines · Peter Zijlstra
Quoted 15 lines · Aravinda Prasad
Current tracing infrastructure such as perf and ftrace reports system wide data when invoked inside a container. It is required to restrict events specific to a container context when such tools are invoked inside a container.
This RFC patch supports filtering container specific events, without any change in the user interface, when invoked within a container for the perf utility; such support needs to be extended to ftrace. This patch assumes that the debugfs is available within the container and all the processes running inside a container are grouped into a single
perf_eventsubsystem of cgroups. This patch piggybacks on the existing support available for tracing with cgroups [1] by setting the cgrp member of the event structure to the cgroup of the context perf tool is invoked from.
Quoted 8 lines
Quoted 5 lines
However, this patch is not complete and requires more work to fully support tracing inside a container. This patch is intended to initiate the discussion on having container-aware tracing support. A detailed explanation on what is supported and pending issues are mentioned below.
tracing is outside the scope of perf; I suspect you want tracefs to be sensitive to filesystem namespaces and all that that entails.
I'd correct that to:
Quoted 2 lines
ftrace is outside the scope of perf; I suspect you want tracefs to be sensitive to filesystem namespaces and all that that entails.
because perf very much does tracing as well, we have 'perf trace' for example, and obviously the whole ring-buffer is a trace buffer and perf.data is a trace dump of that.
Thanks,
Ingo