Fix duplicate /proc/<pid>/root prefix bug that formed wrong path to libssl.so files.
#1740
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Pete Stevenson <jps@pixielabs.ai>
added 2 commits
October 17, 2023 12:00
Signed-off-by: Pete Stevenson <jps@pixielabs.ai>
etep
changed the title
/proc/<pid>/root prefix bug that prevented us from finding libssl.so files.
Signed-off-by: Pete Stevenson <jps@pixielabs.ai>
etep
changed the title
/proc/<pid>/root prefix bug that prevented us from finding libssl.so files./proc/<pid>/root prefix bug that formed wrong path to libssl.so files.
ddelnano
approved these changes
Oct 17, 2023
There was a problem hiding this comment.
@etep thanks for tracking this down and fixing the issue.
I think we should include a Changelog Message: in the PR explaining that a bug was fixed that prevented dynamically linked OpenSSL tracing from working. In addition to that, can you update the Title: to match the PR's.
JamesMBartlett
approved these changes
Nov 2, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary: Fixes a bug that prevented SSL tracing on dynamically loaded SSL libraries. When probing open ssl libs found from a pod's mapped libraries, we erroneously prefixed library paths with
/proc/<pid>/roottwice, i.e. such that a target library might look like this:Normal target processes run in their own pid namespace. This meant that the second instance of
<pid>in the path above would not exist in the target process namespace. Hence probing dynamic libraries using this path failed.We also update the open-ssl test case to not use host pid namespace, i.e. such that those tests expose this bug. Previously, this bug was masked in those test cases because the test container processes ran with host pid namespace.
Related issues: #1736.
Type of change: /kind bug fix.
Test Plan: Show that tests fail when we disable use of host pid namespace in SSL test containers, see #1744.
Changelog Message: