linux-kernel - Re: [ocfs2] c42ff46f97: sysctl_table_check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87a6ffst4j.fsf@email.froward.int.ebiederm.org>
Date:   Fri, 28 Jan 2022 10:49:16 -0600
From:   "Eric W. Biederman" <ebiederm@...ssion.com>
To:     Jan Kara <jack@...e.cz>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        kernel test robot <oliver.sang@...el.com>,
        Luis Chamberlain <mcgrof@...nel.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        Amir Goldstein <amir73il@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Antti Palosaari <crope@....fi>, Arnd Bergmann <arnd@...db.de>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Benjamin LaHaise <bcrl@...ck.org>,
        Clemens Ladisch <clemens@...isch.de>,
        David Airlie <airlied@...ux.ie>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Iurii Zaikin <yzaikin@...gle.com>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Joel Becker <jlbec@...lplan.org>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Joseph Qi <joseph.qi@...ux.alibaba.com>,
        Julia Lawall <julia.lawall@...ia.fr>,
        Kees Cook <keescook@...omium.org>,
        Lukas Middendorf <kernel@...force.de>,
        Mark Fasheh <mark@...heh.com>, Paul Turner <pjt@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Petr Mladek <pmladek@...e.com>,
        Phillip Potter <phil@...lpotter.co.uk>,
        Qing Wang <wangqing@...o.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        Sebastian Reichel <sre@...nel.org>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        Stephen Kitt <steve@....org>,
        Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        Xiaoming Ni <nixiaoming@...wei.com>,
        Douglas Gilbert <dgilbert@...erlog.com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        Jani Nikula <jani.nikula@...el.com>,
        John Ogness <john.ogness@...utronix.de>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Suren Baghdasaryan <surenb@...gle.com>,
        Theodore Ts'o <tytso@....edu>,
        Andrew Morton <akpm@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>, lkp@...ts.01.org,
        kernel test robot <lkp@...el.com>
Subject: Re: [ocfs2] c42ff46f97: sysctl_table_check_failed

Jan Kara <jack@...e.cz> writes:

> On Fri 28-01-22 10:00:29, Linus Torvalds wrote:
>> On Fri, Jan 28, 2022 at 8:53 AM kernel test robot <oliver.sang@...el.com> wrote:
>> >
>> > commit: 46e33fd45a52bf03769906e64d8a8a1ab317777d ("ocfs2: simplify subdirectory
>> > registration with register_sysctl()")
>> 
>> Well, it's apparently commit c42ff46f97c1 ("ocfs2: simplify
>> subdirectory registration with register_sysctl()") in mainline now.
>> 
>> What worries me a bit is that the commit was auto-generated, and when
>> reading the commit message it reads as if it wasn't supposed to cause
>> any semantic changes at all.
>> 
>> Is the cause of this that 'nm' is supposed to be a directory, and
>> register_sysctl() doesn't handle directories?
>> 
>> I don't know this code at all, should it have been simplified even
>> further with something (TOTALLY UNTESTED) like the attached?
>
> Yep, I've tested the patch and it fixes the failure for me. Feel free to
> add:
>
> Tested-by: Jan Kara <jack@...e.cz>
>
> Also the change makes sense to me as far as I'm reading register_sysctl()
> so you can also add:
>
> Reviewed-by: Jan Kara <jack@...e.cz>

Yes.  There is a register_sysctl_paths that can be used if you want/need
the embedded directories.  That probably would have been a better
choice for an automated conversion.

But since this there is only the single file in a single directory
register_sysctl() with the full path is perfectly fine in this case.

Reviewed-by: "Eric W. Biederman" <ebiederm@...ssion.com>

>> diff --git a/fs/ocfs2/stackglue.c b/fs/ocfs2/stackglue.c
>> index 731558a6f27d..dd77b7aaabf5 100644
>> --- a/fs/ocfs2/stackglue.c
>> +++ b/fs/ocfs2/stackglue.c
>> @@ -661,17 +661,6 @@ static struct ctl_table ocfs2_nm_table[] = {
>>  	{ }
>>  };
>>  
>> -static struct ctl_table ocfs2_mod_table[] = {
>> -	{
>> -		.procname	= "nm",
>> -		.data		= NULL,
>> -		.maxlen		= 0,
>> -		.mode		= 0555,
>> -		.child		= ocfs2_nm_table
>> -	},
>> -	{ }
>> -};
>> -
>>  static struct ctl_table_header *ocfs2_table_header;
>>  
>>  /*
>> @@ -682,7 +671,7 @@ static int __init ocfs2_stack_glue_init(void)
>>  {
>>  	strcpy(cluster_stack_name, OCFS2_STACK_PLUGIN_O2CB);
>>  
>> -	ocfs2_table_header = register_sysctl("fs/ocfs2", ocfs2_mod_table);
>> +	ocfs2_table_header = register_sysctl("fs/ocfs2/nm", ocfs2_nm_table);
>>  	if (!ocfs2_table_header) {
>>  		printk(KERN_ERR
>>  		       "ocfs2 stack glue: unable to register sysctl\n");