Skip to content

gettext: backport upstream memory safety fix#423179

Merged
K900 merged 1 commit intoNixOS:stagingfrom
alyssais:gettext-musl
Jul 7, 2025
Merged

gettext: backport upstream memory safety fix#423179
K900 merged 1 commit intoNixOS:stagingfrom
alyssais:gettext-musl

Conversation

@alyssais
Copy link
Member

@alyssais alyssais commented Jul 7, 2025

This fixes building perlPackages.Po4a on musl. While we haven't seen it with Glibc, I have no reason to believe the bug is musl-specific.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • 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/)
  • Nixpkgs 25.11 Release Notes (or backporting 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 25.05 NixOS Release notes)
    • (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, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

@alyssais
gettext: backport upstream memory safety fix
fc7352a 
This fixes building perlPackages.Po4a on musl.  While we haven't seen
it with Glibc, I have no reason to believe the bug is musl-specific.

Fixes: 74042ae ("gettext: 0.22.5 -> 0.25")
@alyssais alyssais mentioned this pull request Jul 7, 2025
Merged
13 tasks
@nix-owners nix-owners bot requested a review from zimbatm July 7, 2025 11:44
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Jul 7, 2025
@LordGrimmauld
Copy link
Contributor

LordGrimmauld commented Jul 7, 2025 •
edited
Loading

Has this been merged upstream yet? A link to the source of the patch and where to track its progress would be good.

This patch looks larger than it needs to be. Assuming the issues are use-after-free, then calling sb_free(&buffer); after savable_comment_add (...); (instead of before) should be all that is needed to fix this, likely no need to introduce that extra intermediary variable. But then again these issues are inherently finnicky, especially if they only sometimes happen. That said, if this ends up the official patch then i'll accept whatever upstream decides to merge.

@alyssais
Copy link
Member Author

alyssais commented Jul 7, 2025

Has this been merged upstream yet? A link to the source of the patch and where to track its progress would be good.

You mean like the X-Git-Url header pointing to the upstream repo in the patch file?

@LordGrimmauld
Copy link
Contributor

LordGrimmauld commented Jul 7, 2025

Ah oops i am blind, sorry

@LordGrimmauld
Copy link
Contributor

LordGrimmauld commented Jul 7, 2025

Running a build to confirm this fixes Po4a on musl, but other than that pending sanity check this change looks fine

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jul 7, 2025
@LordGrimmauld
Copy link
Contributor

LordGrimmauld commented Jul 7, 2025

I confirmed gettext pkgsMusl.gettext perl540Packages.Po4a pkgsMusl.perl540Packages.Po4a all build. With the patch being recognized upstream, this looks good to me.

@LordGrimmauld
Copy link
Contributor

LordGrimmauld commented Jul 7, 2025

Might conflict with #422916, but seeing as you did both PRs i'll trust you know what you are doing in terms of resolving conflicts with yourself.

@K900 K900 merged commit d597d4f into NixOS:staging Jul 7, 2025
28 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LordGrimmauld LordGrimmauld LordGrimmauld approved these changes

@K900 K900 Awaiting requested review from K900

@wegank wegank Awaiting requested review from wegank

@zimbatm zimbatm Awaiting requested review from zimbatm

+1 more reviewer

@MisileLab MisileLab MisileLab approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@alyssais @LordGrimmauld @MisileLab @K900