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.

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

Openafs security updates #48571

Merged
merged 4 commits into from Oct 18, 2018
Merged

Openafs security updates #48571

merged 4 commits into from Oct 18, 2018

Conversation

spacefrogg
Copy link
Contributor

@spacefrogg spacefrogg commented Oct 16, 2018

Motivation for this change

New packages for openAFS fixing security bugs.

The openafs-1.8.2 release also adds support for ARM64.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

AC_SUBST([PATH_CPP])
diff -Nur --unidirectional-new-file openafs-1.8.2/src/cf/ax_prog_cc_for_build.m4 openafs-1.8.2.new/src/cf/ax_prog_cc_for_build.m4
--- openafs-1.8.2/src/cf/ax_prog_cc_for_build.m4 1970-01-01 01:00:00.000000000 +0100
+++ openafs-1.8.2.new/src/cf/ax_prog_cc_for_build.m4 2018-10-16 16:20:40.278641658 +0200
Copy link
Member

Choose a reason for hiding this comment

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

Where is this coming from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a macro from the GNU Autoconf Archive (see also in the header of the file).

Copy link
Member

Choose a reason for hiding this comment

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

Would autoreconfHook also update the file in question or is this something that openafs has vendored?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I know, this is something not normally distributed with autoconf but an additional archive of contributed macros.

@spacefrogg
Copy link
Contributor Author

spacefrogg commented Oct 17, 2018

RFC

In the current PR I just advanced the compatibility bound to the next kernel major version. This means the package upgrade will break at the user side when that kernel becomes the default on their machine. In the PRs before, I removed the upper compatibility check with every upgrade, so that it fails loud and early with the appearance of a new kernel, long before it becomes the default on end user machines.

Which is the better way to do things? Failing early can be an annoyance, too, because openafs normally needs some weeks (even months) to become compatible with the latest kernel release.

@Mic92
Copy link
Member

Mic92 commented Oct 17, 2018

For zfs we usually have no the upper bound if there is no known incompatibility and set it as build errors appear. This means in nixos unstable we sometimes propagate build errors to the users, if we are not marking incompatibilities fast enough. Not sure if it is the best approach. I suppose when somebody actively test the latest kernel with a third-party modules then disabling newer kernels proactively is better because the maintainer would test it before. When we do not have such a maintainer not having the upper bound is better because then the build errors become also apparent in hydra and we can fix it before a release.

@spacefrogg
Copy link
Contributor Author

I will update the PR, then, to not specify an upper bound and follow this approach when there is no known incompatibility at the time of the update.

This release adds support for ARM64.
Add support for 4.18 kernels.
@Mic92
Copy link
Member

Mic92 commented Oct 18, 2018

Somehow we ended up with a openafs reference of a broken version in the manual.

@spacefrogg
Copy link
Contributor Author

The error comes from the nixos client module in nixos/modules/services/network-filesystems/openafs/client.nix:

module = mkOption {
          default = config.boot.kernelPackages.openafs;
          type = types.package;
          description = "OpenAFS kernel module package. MUST match the userland package!";
        };

nix-instantiate does not like the default assignment anymore. Replacing the package name with null fixes this particular issue but destroys the semantics...

broken = versionOlder kernel.version "3.18" ||
versionAtLeast kernel.version "4.18";
broken = versionOlder kernel.version "3.18"
|| stdenv.targetPlatform.isAarch64;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, this line breaks the manual evaluation. But I don't know, why...

Copy link
Member

Choose a reason for hiding this comment

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

Please keep this line as it is and set defaultText in the manual to fix evaluation.

@Mic92
Copy link
Member

Mic92 commented Oct 18, 2018

I think there is also defaultText which is used instead of default so it will not evaluate openafs on aarch64.

@Mic92
Copy link
Member

Mic92 commented Oct 18, 2018

backport: cb996fd

@andersk andersk mentioned this pull request Dec 7, 2019
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants