Skip to content

Allow link.OpenExecutable for files without executable bit set#1969

Open
shaunduncan wants to merge 1 commit intocilium:mainfrom
speedscale:fix/1964
Open

Allow link.OpenExecutable for files without executable bit set#1969
shaunduncan wants to merge 1 commit intocilium:mainfrom
speedscale:fix/1964

Conversation

@shaunduncan
Copy link
Copy Markdown
Contributor

This change removes the restriction added in PR #1938 that requires file paths given to link.OpenExecutable() have the executable bit set. This restores the previous behvaior in versions prior to v0.21.0 while also preserving the new capabilities introduced in PR #1938.

This should resolve #1964

mmat11
mmat11 previously approved these changes Mar 23, 2026
ti-mo
ti-mo previously approved these changes Mar 24, 2026
Copy link
Copy Markdown
Contributor

@ti-mo ti-mo left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up! Please squash into one commit when you've addressed the feedback.

This change removes the restriction added in PR cilium#1938 that requires file paths
given to link.OpenExecutable() have the executable bit set. This restores the
previous behvaior in versions prior to v0.21.0 while also preserving the new
capabilities introduced in PR cilium#1938.

Signed-off-by: Shaun Duncan <shaun@speedscale.com>
@shaunduncan
Copy link
Copy Markdown
Contributor Author

Hi everyone - I've addressed the feedback, squashed the branch, and re-pushed. Thanks for taking a look!

@mmat11
Copy link
Copy Markdown
Contributor

mmat11 commented Mar 24, 2026

@shaunduncan I believe there's some related test code failing still

Copy link
Copy Markdown

@skymensch skymensch left a comment

Choose a reason for hiding this comment

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

Hey @shaunduncan, the fix looks correct conceptually, but link/uprobe_test.go also needs to be updated to match the new behavior.

Currently, TestExecutable creates a file with mode 0600 and expects OpenExecutable() to return an error:

// uprobe_test.go ~line 41
_, err = OpenExecutable(path)
if err == nil {
    t.Fatal("create executable: expected error on non-executable file")
}

Since this PR removes that check, OpenExecutable() now succeeds on a 0600 file — so the test fails at that assertion.

The fix is to replace that block (and the subsequent os.Chmod step) with an assertion that expects success:

// OpenExecutable should succeed regardless of execute bit
_, err = OpenExecutable(path)
if err != nil {
    t.Fatalf("create executable: unexpected error on non-executable file: %v", err)
}

That should make the CI green. 🙂

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.

OpenExecutable permission check prevents uprobe attachment to shared libraries without execute bit

4 participants