On Wed, Feb 9, 2022 at 2:58 PM Luis Chamberlain <mcgrof@xxxxxxxxxx> wrote:
>
> This is the second attempt to move binfmt_misc sysctl to its
> own file. The issue with the first move was that we moved
> the binfmt_misc sysctls to the binfmt_misc module, but the
> way things work on some systems is that the binfmt_misc
> module will load if the sysctl is present. If we don't force
> the sysctl on, the module won't load. The proper thing to do
> is to register the sysctl if the module was built or the
> binfmt_misc code was built-in, we do this by using the helper
> IS_ENABLED() now.
>
> The rationale for the move:
>
> kernel/sysctl.c is a kitchen sink where everyone leaves their dirty
> dishes, this makes it very difficult to maintain.
>
> To help with this maintenance let's start by moving sysctls to places
> where they actually belong. The proc sysctl maintainers do not want to
> know what sysctl knobs you wish to add for your own piece of code, we
> just care about the core logic.
>
> This moves the binfmt_misc sysctl to its own file to help remove clutter
> from kernel/sysctl.c.
>
> Cc: Domenico Andreoli <domenico.andreoli@xxxxxxxxx>
> Cc: Tong Zhang <ztong0001@xxxxxxxxx>
> Reviewed-by: Tong Zhang <ztong0001@xxxxxxxxx>
> Signed-off-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
> ---
>
> Andrew,
>
> If we get tested-by from Domenico and Tong I think this is ready.
>
> Demenico, Tong, can you please test this patch? Linus' tree
> should already have all the prior work reverted as Domenico requested
> so this starts fresh.
>
> fs/file_table.c | 2 ++
> kernel/sysctl.c | 13 -------------
> 2 files changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/fs/file_table.c b/fs/file_table.c
> index 57edef16dce4..4969021fa676 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -119,6 +119,8 @@ static struct ctl_table fs_stat_sysctls[] = {
> static int __init init_fs_stat_sysctls(void)
> {
> register_sysctl_init("fs", fs_stat_sysctls);
> + if (IS_ENABLED(CONFIG_BINFMT_MISC))
> + register_sysctl_mount_point("fs/binfmt_misc");
> return 0;
> }
> fs_initcall(init_fs_stat_sysctls);
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 241cfc6bc36f..788b9a34d5ab 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2735,17 +2735,6 @@ static struct ctl_table vm_table[] = {
> { }
> };
>
> -static struct ctl_table fs_table[] = {
> -#if defined(CONFIG_BINFMT_MISC) || defined(CONFIG_BINFMT_MISC_MODULE)
> - {
> - .procname = "binfmt_misc",
> - .mode = 0555,
> - .child = sysctl_mount_point,
> - },
> -#endif
> - { }
> -};
> -
> static struct ctl_table debug_table[] = {
> #ifdef CONFIG_SYSCTL_EXCEPTION_TRACE
> {
> @@ -2765,7 +2754,6 @@ static struct ctl_table dev_table[] = {
>
> DECLARE_SYSCTL_BASE(kernel, kern_table);
> DECLARE_SYSCTL_BASE(vm, vm_table);
> -DECLARE_SYSCTL_BASE(fs, fs_table);
> DECLARE_SYSCTL_BASE(debug, debug_table);
> DECLARE_SYSCTL_BASE(dev, dev_table);
>
> @@ -2773,7 +2761,6 @@ int __init sysctl_init_bases(void)
> {
> register_sysctl_base(kernel);
> register_sysctl_base(vm);
> - register_sysctl_base(fs);
> register_sysctl_base(debug);
> register_sysctl_base(dev);
>
> --
> 2.34.1
>
Hi Luis,
Thanks for posting.
I checked the master branch just now and the fix is already in, see
commit b42bc9a3c511("Fix regression due to "fs: move binfmt_misc
sysctl to its own file"")
I have tested it yesterday on a debian machine and it appears to be ok.
- Tong
