On Wed, Feb 09, 2022 at 03:15:53PM -0800, Tong Zhang wrote:
> 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.
The "fix" was to vert the original effort. This patch continues with the
effort and does it properly. As such it is a change which needs to be
tested. I'd appreciate if you can test.
Luis
