On Wed, Feb 9, 2022 at 3:29 PM Luis Chamberlain <mcgrof@xxxxxxxxxx> wrote:
>
> 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
Tested-by: Tong Zhang<ztong0001@xxxxxxxxx>
