bash: update patches

[ju1m]

Motivation for this change

Apply latest fixes to Bash 5.1. Especially 009, fixing a bug in its malloc implementation.
I've merged against master because those are fixes, but feel free to argue that it should be against staging.

Things done

• Run pkgs/shells/bash/update-patch-set.sh bash 4.4 --> nothing new
• Run pkgs/shells/bash/update-patch-set.sh bash 5.1

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 21.11 Release Notes (or backporting 21.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

[martinetd]

test failure is as follow:
error: unable to download 'https://ftpmirror.gnu.org/bash/bash-5.1-patches/bash51-012': HTTP error 404

I can confirm I have the same error, ftpmirror around the world are probably just not updated yet? it finds up to 008 just fine.

[happysalada]

I agree with merging to master, since we are cutting it close to the 21.11 release though, let me see what the release managers have to say about it.
I've just checked and I don't get the 404 anymore, perhaps force push the branch to retrigger the build ?

[picnoir]

I can confirm I have the same error, ftpmirror around the world are probably just not updated yet? it finds up to 008 just fine.

I think that's the case, 012 seems to be resolved without any issue on my end now.

@GrahamcOfBorg eval

[vcunat]

Surely not directly to master, as it rebuilds completely everything. staging-next is conceivable at this moment (and gets to 21.11 automatically), but I think that's worth it only if the fix is very urgent... and I don't see that indicated. Otherwise I'd expect staging and then a bit later backport to 21.11.

[ju1m]

@vcunat, my motivation for 009 was to be able to use DynamicUser= in some NixOS services without workaround as this one for syncoid:

BindReadOnlyPaths = [ builtins.storeDir "/etc" "/run" "/bin/sh"
  # A custom LD_LIBRARY_PATH is needed to access in `getent passwd`
  # the systemd's entry about the DynamicUser=,
  # so that ssh won't fail with: "No user exists for uid $UID".
  #
  # Unfortunately, Bash 5.1 is incompatible with libnss_systemd.so:
  # https://www.mail-archive.com/bug-bash@gnu.org/msg24306.html
  # Hence we cannot just directly put this custom LD_LIBRARY_PATH
  # in the service's environment as one would expect
  # but have to use a workaround: wrapping ssh.
  # This wrapping is done here as a mounted path because
  # because Nixpkgs' wrapping of syncoid enforces the use
  # of the ${pkgs.openssh}/bin/ssh path.
  # This problem does not arise on NixOS systems where stdenv.hostPlatform.libc == "musl",
  # because then Bash is built with --without-bash-malloc
  # This problem will be fixed once https://ftp.gnu.org/gnu/bash/bash-5.1-patches/bash51-009
  # has been applied to Bash.
  ("${pkgs.writeShellScript "ssh-with-support-for-DynamicUser" ''
    export LD_LIBRARY_PATH="${config.system.nssModules.path}"
    exec -a ${pkgs.openssh}/bin/ssh /bin/ssh "$@"
  ''}:${pkgs.openssh}/bin/ssh")

  "${pkgs.openssh}/bin/ssh:/bin/ssh"
];

So at least there is a workaround in this case, but knowing that this bug allows to "overwrite memory bounds checking" it sounds like it may be exploitable, though I don't have any proof of concept to back that up. Therefore I would prefer to have those patches included as soon as possible. To this end I'm rebasing against staging-next as you suggested.

[SuperSandro2000]

About merging this to master with the rebuild amount: this would only be possible if it would include a very high severity CV or bug that needs to be patched asap and is or likely is to be exploited in the wild with very high damage. For example if something gets a CVE of 10.0 we could justify it. This can of course be expanded and if there is a good argument Something can be always merged to master which I don't see here.

[ju1m]

@jonringer, I've rebased on staging as you requested.

[jonringer]

@ofborg eval

[happysalada]

tests seem to have failed on a subtest of git 2.34
t2200-add-update.sh ................................ Dubious, test returned 1 (wstat 256, 0x100)
https://github.com/git/git/blob/master/t/t2200-add-update.sh
I can't really see anything in that test that seems bash specific. I'm going to try to do the eval again just to see if it is a flaky test.

[happysalada]

@GrahamcOfBorg eval

[happysalada]

re-evaluation doesn't trigger a full eval.
Should we still try to merge on staging and potentially disable the flaky test there ?

[jonringer]

tests seem to have failed on a subtest of git 2.34 t2200-add-update.sh ................................ Dubious, test returned 1 (wstat 256, 0x100) https://github.com/git/git/blob/master/t/t2200-add-update.sh I can't really see anything in that test that seems bash specific. I'm going to try to do the eval again just to see if it is a flaky test.

I was able to get it passing locally with --cores 1

[happysalada]

So are you ok to merge as is, or would you prefer we "enableParallelBuild = false;" ?

[vcunat]

Succeeds for me with 32 cores. I retried a couple times. Jon's using some extreme machine, I suspect.

[happysalada]

Ok then let's consider this test flaky and think about disabling it potentially if this is still a problem in staging-next.

[SuperSandro2000]

Succeeds for me with 32 cores. I retried a couple times. Jon's using some extreme machine, I suspect.

128 cores IIRC