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.

Sign up for GitHub

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

Jump to bottom

ayatana-indicator-messages: fix PIE hardening #372768

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

ayatana-indicator-messages: fix PIE hardening #372768

wants to merge 1 commit into from

Conversation

FliegendeWurst
Copy link
Member

@FliegendeWurst FliegendeWurst commented Jan 10, 2025 •
edited
Loading

ref. #205031

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Jan 10, 2025
@nix-owners nix-owners bot requested a review from OPNA2608 January 10, 2025 22:28
@OPNA2608
Copy link
Contributor

OPNA2608 commented Jan 11, 2025 •
edited
Loading

Failure seems to be due to these hardcoded -no-pie flags for linking two of the test executables. I'll check if upstream could just drop these, as building with and without hardeningEnable = [ "pie" ] seems to work fine after dropping these.

Edit: Suggested in AyatanaIndicators/ayatana-indicator-messages#39 .

@OPNA2608 OPNA2608 self-assigned this Jan 11, 2025
@OPNA2608 OPNA2608 mentioned this pull request Jan 11, 2025
Open
@FliegendeWurst FliegendeWurst changed the title ayatana-indicator-messages: disable PIE hardening ayatana-indicator-messages: fix PIE hardening Jan 11, 2025
@FliegendeWurst
Copy link
Member Author

Thanks for investigating and fixing the issue :)
Picked up your patch.

OPNA2608
OPNA2608 reviewed Jan 11, 2025
View reviewed changes
pkgs/by-name/ay/ayatana-indicator-messages/package.nix
@@ -40,6 +40,10 @@ stdenv.mkDerivation (finalAttrs: {
"dev"
] ++ lib.optionals withDocumentation [ "devdoc" ];

patches = [
./fix-pie.patch
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to not just fetchpatch it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. It is not yet merged, so it could disappear if you were to force-push to the PR branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

Alrighty, section got added since the last time I had a deep look at pkgs/CENTRIBUTING.md that properly details when & why to do this: https://github.com/NixOS/nixpkgs/blob/2c8f8a719a9018a43984062272ebf5772dc786d6/pkgs/README.md#vendoring-patches

Are we in a rush to get this resolved? Otherwise I'd just wait abit for upstream to respond to the PR, to see if we can avoid adding the patch to Nixpkgs.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a chance PIE will be enabled for the next staging cycle. In that case we have about 5-6 weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OPNA2608 OPNA2608 OPNA2608 left review comments

Assignees

@OPNA2608 OPNA2608

Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@FliegendeWurst @OPNA2608