Linux: Fix openzfs kernel module loading on 6.11+ with CONFIG_RANDSTRUCT=y

[IvanVolosyuk]

Fixes openzfs kernel module loading on 6.11+ with CONFIG_RANDSTRUCT=y

Motivation and Context

This change fixes #16620
It looks like detection logic for register_sysctl_sz() in kernel-register_sysctl_table.m4 doesn't quite work with CONFIG_RANDSTRUCT=y. It causes the same issue as before the original fix in 2633075#diff-cc1bb9d36ff95af7e87e3c4a8fc022ea302ecfdccbc9fa1e9663c9c7b51dc416R722

sysctl table check failed: kernel/spl/(null) No proc_handler

Description

The fix is to replace {0} with {{}} in the m4 script, which better matches the sentinels we use in spl-proc.c

How Has This Been Tested?

• Verified that register_sysctl_sz is detected correctly on kernels v6.5, v6.10, v6.11.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[amotin]

I bisected the kernel commit where the issue starts to occur:

This is the commit when it stopped allow the sentinels. Instead I think you could dig deeper to find when they become optional, since that is where the KPI has changed, and I wonder if it was more documented back then. The later was just a compatibility window.

[snajpa]

Kernel versions aren't reliable, stuff can be backported to older kernels (RHEL kernels are known for that), testing a kernel version within OpenZFS code to make a decision of which code to use is thus not a good idea. It always needs a specific configure test, which affects zfs_config.h, which is what you can then use in the source to disambiguate between the different variants.

(I don't speak m4 either, I always just go looking for something in the config/ dir that is the closest to what I'm trying to do and then just copy and modify it)

[snajpa]

btw how about, instead of ifdefing everywhere, define a macro that will either fill in the optional guard or nothing? something like

#ifdef HAVE_PROC_GUARDS
#define OPTIONAL_PROC_GUARD {}
#else
#define OPTIONAL_PROC_GUARD
#endif

and then just place OPTIONAL_PROC_GUARD at the right spots

[IvanVolosyuk]

Thank you, guys, for the feedback. I dig a bit more into this and made a facepalm.

It turns out @robn already did what I was trying to do in a more elegant way:
2633075#diff-cc1bb9d36ff95af7e87e3c4a8fc022ea302ecfdccbc9fa1e9663c9c7b51dc416R722

The real problem to be solved is: why CONFG_RANDSTRUCT breaks the detection of register_sysctl_sz() in config/kernel-register_sysctl_table.m4
I can confirm that with RANDSTRUCT the HAVE_REGISTER_SYSCTL_SZ is undefined on v6.11.

[IvanVolosyuk]

I have updated the commit and pull request description. It ended up being just a one-liner.

[robn]

If it works, then it works for me. Good stuff!

Do you happen to have the output from the failing register_sysctl_sz test? And which compiler you're using? I'm curious about how it failed, that this could fix it. I'm suspecting a version of #16206.

[IvanVolosyuk]

Sure, @robn
I'm using gcc (Gentoo 13.3.1_p20241115 p1) 13.3.1 20241115
Here is the output from the test I was getting:

/sources/zfs/build/has_register_sysctl_sz/has_register_sysctl_sz.c: In function ‘main’:
/sources/zfs/build/has_register_sysctl_sz/has_register_sysctl_sz.c:103:74: error: invalid initializer
  103 |                 struct ctl_table test_table[] __attribute__((unused)) = {0};
      |                                                                          ^
/sources/zfs/build/has_register_sysctl_sz/has_register_sysctl_sz.c:103:74: note: (near initialization for ‘test_table[0].<anonymous>’)
make[3]: *** [scripts/Makefile.build:244: /sources/zfs/build/has_register_sysctl_sz/has_register_sysctl_sz.o] Error 1
make[3]: Target '/sources/zfs/build/has_register_sysctl_sz/' not remade because of errors.
make[2]: *** [scripts/Makefile.build:485: /sources/zfs/build/has_register_sysctl_sz] Error 2

[robn]

Oh right, much dumber than I thought. The 0 is applied to the first element. If its a simple scalar, fine, if not, blam. Ok, we all good. Thanks for running it down :)

[IvanVolosyuk]

Damn, I accidentally pushed 'Update branch' button and it created a merge commit to master on top of my changes. To get rid of this I just force pushed to go back to a good old state. So, should I just wait now for merge or I have to do something? e.g. manually rebase on top current master? That's my first commit for openzfs.

[amotin]

@IvanVolosyuk If your master was reasonably recent, that should be fine. Sure the newer the better, but within reason, to not over-stress CI with extra testing. Once all the CI tests complete successfully and there are enough qualified positive reviews, it will be merged.

[IvanVolosyuk]

zloop run was successful before I rebased: https://github.com/openzfs/zfs/actions/runs/12028052804
Now it failed 2 times in a row.

[amotin]

zloop run was successful before I rebased. Now it failed 2 times in a row.

@IvanVolosyuk Unfortunately it is too random and so unstable in its results. It is not necessary that the reports are wrong, just likely not related to this PR. It is not a blocker here.

[amotin]

@IvanVolosyuk It seems github stuck in "Checking for ability to merge automatically…". I can't merge it. Could you rebase and force-push it?

[IvanVolosyuk]

Done, should be good now.

[cawilliamson]

On NixOS despite including this patch I still get the same boot error on the hardened 6.11 kernel.

Anyone else been looking into this and found a solution / workaround?

[AllKind]

@cawilliamson
Please open an new issue in the issue tracker.

• Including all the information in the issue template.
• How the patch was applied.
• And if possible the config.log from the configure process.

[IvanVolosyuk]

Did you run ./autogen.sh after applying the patch? The change is only used when run ./autogen.sh before ./configure as it changes the configure script itself.

[AllKind]

@IvanVolosyuk The issue is now open at: #16849 (closed pull requests don't get much attention...)
I suspected something like you do. But I don't know anything about NixOS and a quick search on their build process lead to nothing. So I didn't comment any further.