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

Move AT_EXECFN to fallback for /proc/self/exe #78958

Merged
merged 2 commits into from
Nov 30, 2022

Conversation

am11
Copy link
Member

@am11 am11 commented Nov 29, 2022

In #66874, the missing header was added which enabled the fast (aux vector) path on Linux.

The issue with AT_EXECFN is that it returns the path which was passed to the process via namei. This breaks the scenario where the .NET process is used as interpreter (namely PowerShell's #!/bin/pwsh) and returns the path of script containing the shebang link to the interpreter rather than the interpreter's path. In other words, AT_EXCFN implementation != /proc/self/exe. In all the usages of minipal_getexepath() in runtime repo, we need the behavior of latter on Linux.

The fix is to remove the usage of AT_EXCFN and rely solely on /proc/self/exe for Linux (which we were accidentally using in .NET 6 due to the missing header).

The fix is to flip the order so AT_EXCFN is used as a fallback to /proc/self/exe.

Fixes #78941 (we will probably need to backport this to .NET 7)

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Nov 29, 2022
@ghost
Copy link

ghost commented Nov 29, 2022

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

In #66874, the missing header was added which enabled the fast (aux vector) path on Linux.

The issue with AT_EXECFN is that it returns the path which was passed to the process via namei. This breaks the scenario where the .NET process is used as interpreter (namely PowerShell's #!/bin/pwsh) and returns the path of script containing the shebang link to the interpreter rather than the interpreter's path. In other words, AT_EXCFN implementation != /proc/self/exe. In all the usages of minipal_getexepath() in runtime repo, we need the behavior of latter on Linux.

The fix is to remove the usage of AT_EXCFN and rely solely on /proc/self/exe for Linux (which we were accidentally using in .NET 6 due to the missing header).

Fixes #78941 (we will probably need to backport this to .NET 7)

Author: am11
Assignees: -
Labels:

area-Host

Milestone: -

@janvorli
Copy link
Member

This is unfortunate w.r.t. asks on minimizing usage of the /proc file system (one example is #2534) or issues where /proc/self/exe doesn't actually return the "real" exe path (#47280). I wonder if there is a way to handle the case this PR is fixing differently.

@janvorli
Copy link
Member

I have looked at the getauxval doc and figured out a solution that doesn't use the /proc/self/exe and yet doesn't have the problem this issue is fixing:

unsigned long entry = getauxval(AT_ENTRY);
    st = dladdr((void*)entry, &info2);

    if (st != 0)
    {
        printf("From AT_ENTRY: %s\n", info2.dli_fname);
    }

I've created a .sh script with the shebang pointing to my test app with the code above and when I ran it it printed this:

From AT_ENTRY: /home/janvorli/test/testatexecfn/test

@agocke agocke added this to the 7.0.x milestone Nov 30, 2022
@am11
Copy link
Member Author

am11 commented Nov 30, 2022

This is unfortunate w.r.t. asks on minimizing usage of the /proc file system

Yes, that's how we discovered the missing header #66874 (comment) when running a .NET app on a system with broken procfs. After that I sent other PRs to remove the usage of procfs in runtime and sdk repos.

This PR, however, is a correctness fix; both #if HAVE_GETAUXVAL and #else branches should be equivalent, but they are not. /proc/self/exe is not perfect, but it just works on a healthy system. This is a low risk fix because we are reverting to .NET 6 behavior.

AT_ENTRY

Could you point me where the docs mention the AT_ENTRY based solution to get the executable path? I have not found any references where someone is using this approach in production. It is either /proc/self/exe or AT_EXCFN. This thread has some discussion on the pros and cons https://lkml.iu.edu/hypermail/linux/kernel/0808.1/3101.html. Each method comes with its own caveats, so I am not confident to jump right on AT_ENTRY solution (which we have never tested before) for backport fix.

@janvorli
Copy link
Member

@am11 I've found AT_ENTRY documented in the getauxval doc. It is the entry point address, so it must be in the actual executable. So I got the idea of using dladdr that can get the full path to the executable. I have not tried to search for it anywhere, so it is possible that no one is using that.
It is possible that there are some pitfalls, so I would definitely not jump into backporting it until we were sure that it works reliably.

I have actually got an additional idea on fixing this issue. What if we just flipped the order of reading the /proc/self/exe and the AT_EXECFN instead of getting rid of the latter? Then the AT_EXECFN would be a fallback for cases when the /proc/self/exe isn't there.

@am11 am11 force-pushed the feature/minipal/getexepath branch from ad27bfc to 87b691b Compare November 30, 2022 01:00
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@am11 am11 changed the title Remove usage of AT_EXECFN Move AT_EXECFN to fallback for /proc/self/exe Nov 30, 2022
@janvorli
Copy link
Member

All the CI failures are known issues unrelated to this change.

@janvorli janvorli merged commit 8f543a1 into dotnet:main Nov 30, 2022
@am11
Copy link
Member Author

am11 commented Nov 30, 2022

Thank you.

I think we should backport this to .NET 7 (to unblock PowerShell users). cc @agocke, @vitek-karas

@am11 am11 deleted the feature/minipal/getexepath branch November 30, 2022 09:59
@janvorli
Copy link
Member

/backport to release/7.0

@github-actions
Copy link
Contributor

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3586639149

@ghost ghost locked as resolved and limited conversation to collaborators Dec 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Host community-contribution Indicates that the PR has been added by a community member
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

dotnet EXE application doesn't work with shebang on Linux
3 participants