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

Fix snapshot automounting with GrSecurity constify plugin. #884

Closed
wants to merge 1 commit into from

Conversation

maxximino
Copy link
Contributor

./configure erroneously detects absence of dops->d_automount when built against a GrSecurity patched kernel.

Error message found in config.log:

configure:24349: checking whether dops->d_automount() exists
configure:24384: cp conftest.c build && make modules -C /mypool/vm/hardenedchroot/kernelsrc/withzfs/linux-3.4.7/ EXTRA_CFLAGS=-Werror-implicit-function-declaration -Werror M=/mypool/vm/hardenedchroot/kernelsrc/withzfs/zfs/build modpost=true
/mypool/vm/hardenedchroot/kernelsrc/withzfs/zfs/build/conftest.c: In function 'main':
/mypool/vm/hardenedchroot/kernelsrc/withzfs/zfs/build/conftest.c:132:28: error: constified variable 'dops' cannot be local
make[1]: *** [/mypool/vm/hardenedchroot/kernelsrc/withzfs/zfs/build/conftest.o] Error 1
make: *** [module/mypool/vm/hardenedchroot/kernelsrc/withzfs/zfs/build] Error 2

The "dops" variable cannot be a local variable, so it's moved to the global scope.

./configure erroneously detects absence of dops->d_automount when built against a GrSecurity patched kernel.

Error message found in config.log:

configure:24349: checking whether dops->d_automount() exists
configure:24384: cp conftest.c build && make modules -C /mypool/vm/hardenedchroot/kernelsrc/withzfs/linux-3.4.7/ EXTRA_CFLAGS=-Werror-implicit-function-declaration -Werror  M=/mypool/vm/hardenedchroot/kernelsrc/withzfs/zfs/build modpost=true
/mypool/vm/hardenedchroot/kernelsrc/withzfs/zfs/build/conftest.c: In function 'main':
/mypool/vm/hardenedchroot/kernelsrc/withzfs/zfs/build/conftest.c:132:28: error: constified variable 'dops' cannot be local
make[1]: *** [/mypool/vm/hardenedchroot/kernelsrc/withzfs/zfs/build/conftest.o] Error 1
make: *** [_module_/mypool/vm/hardenedchroot/kernelsrc/withzfs/zfs/build] Error 2

The "dops" variable cannot be a local variable, so it's moved to the global scope.

This test also fails if the prototype of the dentry_operations.d_automount function pointer is changed.

Signed-off-by: Massimo Maggi <massimo@mmmm.it>
@maxximino
Copy link
Contributor Author

@behlendorf I've updated the patch as you've requested. (sorry, push -f to the branch removed also your comment)
However I think that checking the prototype has a drawback:
If the user builds ZFS against a too new kernel
(otherwise autotools would know about the new prototype with an appropriate check)
he would obtain a partially working ZFS module (without snapshot automount support inside .zfs/snapshot) WITHOUT ANY WARNING.
Instead a compile-time error would alert the user that his kernel is too new and some developer should check the changed interfaces, without forgetting automount support.

@behlendorf
Copy link
Contributor

This doesn't seem like totally undesirable behavior. From the users point of view I'd rather have a working build without .zfs snapshot support than nothing at all because of a failed compile. The real issue as you point out of the lack of an warning message. This behavior can actually already happen with the current source if your kernel is too old.

@ryao
Copy link
Contributor

ryao commented Aug 24, 2012

This patch looks good to me.

The topic of silent errors in autotools checks should be in a separate issue, but I think that the autotools checks could be more rigorous. Rather than checking for the new prototype and assuming that it is either that or the old, checks should exist for both. That would enable autotools to fail gracefully stating that it doesn't know how to properly build against the kernel sources.

pcd1193182 pushed a commit to pcd1193182/zfs that referenced this pull request Sep 26, 2023
Bumps [chrono](https://github.com/chronotope/chrono) from 0.4.24 to 0.4.25.
- [Release notes](https://github.com/chronotope/chrono/releases)
- [Changelog](https://github.com/chronotope/chrono/blob/main/CHANGELOG.md)
- [Commits](chronotope/chrono@v0.4.24...v0.4.25)

---
updated-dependencies:
- dependency-name: chrono
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants