Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

link: close tracefs event if perf event cannot be opened #699

Merged
merged 1 commit into from
Jun 8, 2022

Conversation

nashuiliang
Copy link

When getTraceEventID() or openTracepointPerfEvent() returns an error,
tracefs event (e.g. /sys/kernel/debug/tracing/events/), which has
been created by createTraceFSProbeEvent(), is not cleaned up.

Signed-off-by: Chuang Wang nashuiliang@gmail.com

@lmb lmb requested a review from mmat11 June 4, 2022 17:37
Copy link
Contributor

@mmat11 mmat11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember pointing this out in the initial Kprobe implementation, here's the discussion: #265 (comment). I have no strong opinion but prefer cleaning it up /cc @ti-mo

@nashuiliang
Copy link
Author

nashuiliang commented Jun 5, 2022

Thanks, I read the discussion: #265 (comment).

I should briefly describe my scene:

When livepatch and kprobe are using the same kernel function X, tracefs event (e.g. /sys/kernel/debug/tracing/events/), which has been created by createTraceFSProbeEvent(), cannot be enabled by syscall perf_event_open.

Therefore, I should add closeTraceFSProbeEvent() on failed openTracepointPerfEvent().
Hmm, I saw getTraceEventID() as well, and added it by the way, although I thought it might not return an error.

@ti-mo ti-mo force-pushed the bugfix-leak_kprobe_events branch from bbf2481 to d590803 Compare June 6, 2022 13:32
@ti-mo
Copy link
Collaborator

ti-mo commented Jun 6, 2022

@nashuiliang Thanks for the patch! I've force-pushed an alternative solution to your branch that makes sure this is not forgotten in the future.

Is there a way this could be tested? This seems fairly tricky to do.

@ti-mo ti-mo changed the title link/kprobe: tracefs: clear tracefs event link: close tracefs event if perf event cannot be opened Jun 6, 2022
@nashuiliang
Copy link
Author

@ti-mo Wow, cool!

The test method is more complicated. It can be triggered manually, and it is difficult to write test code at the same time.

@lmb
Copy link
Collaborator

lmb commented Jun 7, 2022

I should briefly describe my scene:

When livepatch and kprobe are using the same kernel function X, tracefs event (e.g. /sys/kernel/debug/tracing/events/), which has been created by createTraceFSProbeEvent(), cannot be enabled by syscall perf_event_open.

Interesting! Out of curiosity, which errno do you get from perf_event_open?

@nashuiliang
Copy link
Author

Error: opening tracepoint perf event: device or resource busy.
Errno: -EBUSY

If a livepatch handler is already active on the symbol, the write to
tracefs will succeed, a trace event will show up, but creating the
perf event will fail.

Signed-off-by: Chuang Wang <nashuiliang@gmail.com>
Co-authored-by: Timo Beckers <timo@isovalent.com>
@ti-mo ti-mo force-pushed the bugfix-leak_kprobe_events branch from d590803 to c80b538 Compare June 8, 2022 07:02
@ti-mo ti-mo merged commit 49ebb13 into cilium:master Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants