-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
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
switch-to-configuration not interpreted using perl #53672
Comments
I encountered the same when trying out the new 5.0.0-rc1 (!!) kernel, and it appears to have been resolved when reverting. Does that work for you? If so, then we need to sort out why it's acting this way (config? dunno!).... |
Seems like it is a problem with |
Please report this upstream! I think https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/binfmt_script.c?id=c2315c187fa0d3ab363fdebe22718170b40473e3 is the culprit but don't know enough about the kernel to fix it. If someone is able to, verify that these cases work:
and
|
Both don't work if you run them through Using
Still seems to be a problem on Still occuring on |
This problem also applies to the command-not-found script. Also, for me running the scripts directly from fish (i.e. |
This is what happens if I try to run the file directly from a fish shell:
|
I confirm this bug as well, running |
So I noticed that the Reproduction:
Now remove one
Running both variants of the script on kernel 4.20.5 throws only the second, expected, error. So this looks like userspace-breaking kernel regression, if it affects essential NixOS scripts. Not sure what kernel dev to contact so I'll leave that up to someone else. PS: I'm using fish shell, error messages in bash may differ but the problem remains. |
Looks like they're enforcing the limit that was supposed to already be applied? And indeed looking at our headers I'm seeing:
Scrolling to the end it looks like this is getting attention elsewhere recently as well, I think the commit linked previously is indeed responsible-- or certainly rather relevant (see the commit message!). Not sure where this leaves us.... :/ |
Has anyone contacted upstream yet? Is there a report (maybe from another project) that is relevant to follow? Fixing it in the nixos kernels is probably not enough; kernel 5.x from other distros may still have issues with Nixpkgs stuff. |
Well I made a bug report on bugzilla, hopefully I CC'ed the right guy. |
I've bisected this using this VM test and the commit that causes the regression is torvalds/linux@8099b04. |
I also think this could break more than just our use case, especially because Perl seems to expect that the hashbang is truncated by So even if our bug report doesn't get noticed, I'd suspect that this change will get reverted eventually, hopefully before 5.0 gets its final release. |
4.20.8 maybe has this behavior now too, maybe?
Anyway I did the obvious thing and patched `BINPRM_BUF_SIZE` and things
are good again--at least as long as I don't use non-NixOS machines or
something :P.
dtzWill@8dc14c6
and parent (apologies).
Not suggesting nixpkgs should adopt this but thought I'd share in case
others found things acting strange after a recent update as well ;).
…On Sat, 02 Feb 2019 15:30:26 -0800, aszlig ***@***.***> wrote:
I also think this could break more than just our use case, because while the hashbang is truncated by `BINPRM_BUF_SIZE`. There is a reason why Perl [parses the hashbang and `execve()s` itself](https://perl5.git.perl.org/perl.git/blob/04db542212fdad3a62f13afe741c99028f4bf799:/toke.c#l5524), so that multiple arguments are possible and the length limitation doesn't affect it, so I guess it's expected behavior even on other Unices.
So even if our bug report doesn't get noticed, I'd suspect that this change will get reverted eventually, hopefully before 5.0 gets its final release.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#53672 (comment) part: text/html
|
This might be affecting the current LTS (4.19.21) in addition to 4.20.8 (as reported by @dtzWill) and 5.0-rc*. Assuming the commit tracked down by @aszlig is the right one: Found also in
|
Based on the documentation at https://www.kernel.org/doc/linux/REPORTING-BUGS and some extra courage from @mmlb, we've decided @samueldr will write a message to LKML and Linus. I'll send Oleg (author of that patch) an email now, though. |
Mailing list thread: |
@dtzWill Could we have a PR with the patch? Even if only as a temporary mediation? |
@samueldr I think you may want to CC Andrew Morton [email protected], since he signed off on the commit. I'd like to CC him myself, but then I'd have to figure out how to send e-mails to the mailing list first. |
Actually, one can just send him an e-mail regarding the matter now that I think about it. |
Doesn't fix the problem but fixes my/our machines for now. I'm running 4.20.8 with this presently, other kernels "should" patch okay but I haven't checked. And I wasn't sure if hardening wanted this or not? Dunno. NixOS#53672 (comment)
Did any channel bump including these bad kernels?
…Sent from my iPhone
On Feb 14, 2019, at 08:33, Tim Steinbach ***@***.***> wrote:
Oh, I see... Although from what I can tell, Kees' patch is more or less that
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Only the small channels have seen a bump (just looking at http://howoldis.herokuapp.com/) |
The nixos-unstable-small and nixos-18.09-small channel URLs are being rolled back. |
I can now confirm that #55763 fixes the immediate issue. Built |
Does anybody feel strongly about which fix we apply as long as we have something for the moment? |
I think we should do this multi-pronged:
|
I think the safest choice should be taken, although a proper fix, i.e. this patch is preferable in the end. |
good point @grahamc, but how would you make the shebangs smaller? |
Hydra already does this with buildEnv. Nice perk of this is we get a much faster perl startup time: https://github.com/NixOS/hydra/blob/master/release.nix#L47 |
Somebody make an executive decision or I merge #55763 :D |
I'd like for at least one other person to take a look at #55763 -- kernel upstream or otherwise, but it is my preferred patch. |
Should we leave this one open and pinned for a bit, until channels advance? |
FYI: Our revert patch can be deleted, as upstream has released new, fixed kernels:
:) |
I am on it! |
I didn't know for sure, and when I saw the title of the issue, once pinned, I got cold feet in re-writing the title and editing the main post to make it more useful for external users. Though, I'm thinking it could be a good idea if done. Though it doesn't necessarily needs to be kept open, only visible? |
Let me link the upstream summary of this: https://lwn.net/SubscriberLink/779997/11de2bdc8dbc0d69/ (taken from weekly.nixos.org) |
I've seen this also with NixOS 20.09 and kernel 5.4.77. |
Many issues could end up causing similar behaviour. It might not be that the kernel again decided to stop interpreting the shebang in its entirety. I guess you'll need to have a minimal repro, so we can see what's going on. Shouldn't it fail on Hydra too, if it is a reproducible issue? I don't remember if the issue eluded our tests at the time. Though it's likely to be a new issue, rather than linked to a now year old issue. Or else we'll have to have strong words with the kernel, again. Though, check with the most up to date kernel, I'm running 5.4 past .77 and I haven't had this issue. |
Repro is to deploy a new Hetzner machine with NixOS 20.09. I have only helped a friend get things going, so I didn't spend much time investigating what's going on this time. |
Issue description
Running it as an executable returns a bad interpreter error while running it explicitly with
perl
works fine. Running withbash
also replicates the following error.Steps to reproduce
nixos-rebuild switch
Technical details
Please run
nix-shell -p nix-info --run "nix-info -m"
and paste theresults.
The text was updated successfully, but these errors were encountered: