sysctl proc_handler incompatible type build failure for Linux 6.11.0-061100rc1daily20240802-generic

[solardiz]

make[1]: Entering directory '/usr/src/linux-headers-6.11.0-061100rc1daily20240802-generic'
warning: the compiler differs from the one used to build the kernel
  The kernel was built by: x86_64-linux-gnu-gcc-13 (Ubuntu 13.3.0-3ubuntu1) 13.3.0
  You are using:           gcc-13 (Ubuntu 13.3.0-4ubuntu1) 13.3.0
  CC [M]  /root/src/src/modules/ksyms/p_resolve_ksym.o
  CC [M]  /root/src/src/modules/hashing/p_lkrg_fast_hash.o
  CC [M]  /root/src/src/modules/comm_channel/p_comm_channel.o
  CC [M]  /root/src/src/modules/integrity_timer/p_integrity_timer.o
/root/src/src/modules/comm_channel/p_comm_channel.c:147:25: error: initialization of 'int (*)(const struct ctl_table *, int,  void *, size_t *, loff_t *)' {aka 'int (*)(const struct ctl_table *, int,  void *, long unsigned int *, long long int *)'} from incompatible pointer type 'int (*)(struct ctl_table *, int,  void *, size_t *, loff_t *)' {aka 'int (*)(struct ctl_table *, int,  void *, long unsigned int *, long long int *)'} [-Werror=incompatible-pointer-types]
  147 |       .proc_handler   = p_sysctl_kint_validate,
      |                         ^~~~~~~~~~~~~~~~~~~~~~
/root/src/src/modules/comm_channel/p_comm_channel.c:147:25: note: (near initialization for 'p_lkrg_sysctl_table[0].proc_handler')

[solardiz]

This looks related to Linux commit:

commit 78eb4ea25cd5fdbdae7eb9fdf87b99195ff67508
Author: Joel Granados <j.granados@samsung.com>
Date:   Wed Jul 24 20:59:29 2024 +0200

    sysctl: treewide: constify the ctl_table argument of proc_handlers

    const qualify the struct ctl_table argument in the proc_handler function
    signatures. This is a prerequisite to moving the static ctl_table
    structs into .rodata data which will ensure that proc_handler function
    pointers cannot be modified.

which reminds us that we could want to constify our p_lkrg_sysctl_table as well, perhaps limited to kernels with the above commit? Perhaps some older kernels modify the sysctl tables in-place?

[solardiz]

Adding lots of const indeed makes the build work on latest mainline, but breaks it on most older kernels because we then pass the parameter to proc_dointvec_minmax, which on older kernel expects non-const.

[solardiz]

I'm testing:

/*
 * The change of register_sysctl() from function to macro roughly coincides
 * with constification of the ctl_table argument of proc_handlers.
 */
#ifdef register_sysctl
#define P_STRUCT_CTL_TABLE const struct ctl_table
#else
#define P_STRUCT_CTL_TABLE struct ctl_table
#endif

but somehow this doesn't appear to be exactly right across all of our supported kernels. Looks like I am getting const in there even on some older kernels, so maybe the test that register_sysctl is a macro is also too broad for excluding the sentinel element. If so, we really need to talk to Joel et al. about the way they intend out-of-tree modules to adapt to these changes.

[solardiz]

The above failed these 3 builds:

docker build / build (fedora:latest, dnf install -y kernel-devel kernel gcc make elfutils-libelf-devel) (push) Failing after 1m
[Details](https://github.com/solardiz/lkrg/actions/runs/10256442095/job/28375499012)
docker build / build (alt:sisyphus, apt-get update; apt-get install -y kernel-headers-modules-un-def gcc make li... (push) Failing after 39s
[Details](https://github.com/solardiz/lkrg/actions/runs/10256442095/job/28375499185)
docker build / build (registry.opensuse.org/opensuse/tumbleweed, zypper -n install -y gcc make kernel-default-de... (push) Failing after 48s
[Details](https://github.com/solardiz/lkrg/actions/runs/10256442095/job/28375499329)

so very recent distros/kernels, but not exactly latest.

[solardiz]

The above failed these 3 builds:

I realized this was actually expected per my prior analysis for the sentinel element issue. The function was replaced by a macro a year ago, yet the constification is very recent. I'm now testing a simple version check for 6.11+.

[t-8ch]

which reminds us that we could want to constify our p_lkrg_sysctl_table as well, perhaps limited to kernels with the above commit?

This will only be possible with Linux 6.12 (if everything goes as intended). The changes in 6.11 are only preparation for this.

[solardiz]

Thank you very much for commenting on this, @t-8ch. And for your reply on the lists. So we'll go with 6.11+ and then 6.12+ version checks for these things.

My worry about possible backports to stable was about the whole thing starting with last year's changes. I understand that the recent set of changes builds upon the older ones and would definitely not be backported without those. Maybe I'm over-thinking. Anyway, with Linus' response we have no better option than to wait and see.

[t-8ch]

I understand that the recent set of changes builds upon the older ones and would definitely not be backported without those.

Correct, for both efforts there are a lot of preparation patches in the tree. (Not even all are marked as such, as they are also general cleanups) And I surely don't keep track of those for older kernels.

Maybe I'm over-thinking.

Well, your out-of-tree code does start to look ugly with all those compatibility branches. So I see your point but as upstream developer am happy about not having to care.

In general, if something can be done in a backwards-compatible way, chances are it will be done that way even if only for the sake of the in-tree code.