glibc: 2.38-23 -> 2.38-27 #258972
glibc: 2.38-23 -> 2.38-27 #258972
Conversation
edef1c
added
1.severity: mass-rebuild
edef1c
added
8.has: port to stable
|
Thanks a lot @edef1c ! Currently starting a few test-builds and will do a review+merge when that's done. Hope I'll get everything done today. |
Fixes CVE-2023-4911. Change-Id: I154c79a7c6822051903ef15c547c3567691b1457
delroth
added
the
12.approvals: 1
There was a problem hiding this comment.
With 70cf1cfeb4ce9449dcb3e8d479bab5262d4956b4 (the version before the rename) I did run some builds (nixosTests.nscd and nixosTests.avahi), which should give some exposure into nss lookups still working.
coreutils, libpng-apng-1.6.40 and gnutls were a bit flaky, but ultimately the builds succeeded!
I tried to reproduce the same 2.38-master.patch.gz as @edef1c, but for some reason mine does have a slightly different format (Author and Date, instead of Author[Date] and Commit[Date] fields), but the patch contents are the same.
I'd say let's get this in, we can have a discussion around the patch workflow later.
Same here, but I assumed differences in git versions (and this is non-functional, so 🤷 )
👍 |
That sounds like |
There was a problem hiding this comment.
Why is this a binary checked into git?
There was a problem hiding this comment.
It's explained in common.nix:
/* No tarballs for stable upstream branch, only https://sourceware.org/git/glibc.git and using git would complicate bootstrapping.
$ git fetch --all -p && git checkout origin/release/2.38/master && git describe
glibc-2.38-27-g750a45a783
$ git show --minimal --reverse glibc-2.38.. | gzip -9n --rsyncable - > 2.38-master.patch.gz
To compare the archive contents zdiff can be used.
$ zdiff -u 2.38-master.patch.gz ../nixpkgs/pkgs/development/libraries/glibc/2.38-master.patch.gz
There was a problem hiding this comment.
This explains the need for the patch but not why it's gzipped?
There was a problem hiding this comment.
Because it's already 3KB without being gzipped, and people are careful about not increasing the size of the nixpkgs repo.
There was a problem hiding this comment.
Don't people always download the Nixpkgs repo compressed in some way, though? If you do a git clone, you get compressed packs. If you download a channel, you get a tar.xz. If you download from GitHub, you get a zip or a tar.gz or whatever. I'd expect this file being a .gz to make the compressed size of Nixpkgs as a whole slightly worse, because now it's opaque to the compressor and can't be deduplicated with other similar files.
There was a problem hiding this comment.
We sometimes backport security fixes and tests that validate they work. In some cases the test artifacts are in fact binary (think image formats, asn1, ...), but we'd still like to have them, to ensure the patch works, and does not regress.
This is especially relevant, since fetchpatch drops binary hunks, and even then we sometimes need to put a rebased patch into the tree.
There was a problem hiding this comment.
That sounds different though — the patch itself would still be a text file, right?
There was a problem hiding this comment.
Those artifacts should then either be generated from text-based formats or be fetched from somewhere else.
Binaries in Git are incredibly inefficient in both size and, more importantly, performance and Nixpkgs can already be a challenge in those regards.
There was a problem hiding this comment.
Sorry, I'm a little late to the party 😅
I just skimmed through old PRs and it seems as if we started at around 2.31 or 2.32 with that when we needed to apply a bunch of patches for several issues and decided to just follow the release branch.
I guess it sounds reasonable to switch to a plaintext patchset especially given @alyssais' explanation on the expected influence on nixpkgs tarballs.
cc @TredwellGit @vcunat in case they have anything to add.
There was a problem hiding this comment.
fwiw #287594 contains a patch rather than a gz file.
Announcement: https://lists.gnu.org/archive/html/info-gnu/2024-01/msg00017.html This release seems relatively harmless in terms of potential fallout. Most notably is the removal of `crypt(3)` in favor of libxcrypt which we've done already and compatibility from ISO C2X. Also decided to drop the old *.gz approach in favor of inlining the patch with the changes from the release branch directly: it's relatively small in contrast to certain lockfiles in this repo and having a textual version makes reviews & diffs easier. See also NixOS#258972 (comment) for more context.
Description of changes
Fixes CVE-2023-4911.
Backport to stable: #258975
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)