netdev - Re: [net-next] net: core: use shared sysctl macro

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] [day] [month] [year] [list]

Message-ID: <CAMDZJNVtxNGiSXvGsBqcZ_Xkq+DjH9qzr-NZTUsBmOCZTEqPxA@mail.gmail.com>
Date:   Wed, 16 Mar 2022 23:08:38 +0800
From:   Tonghao Zhang <xiangxia.m.yue@...il.com>
To:     Luis Chamberlain <mcgrof@...nel.org>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Kees Cook <keescook@...omium.org>,
        Iurii Zaikin <yzaikin@...gle.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        David Ahern <dsahern@...nel.org>,
        Simon Horman <horms@...ge.net.au>,
        Julian Anastasov <ja@....bg>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        Jozsef Kadlecsik <kadlec@...filter.org>,
        Florian Westphal <fw@...len.de>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        Lorenz Bauer <lmb@...udflare.com>,
        Akhmat Karakotov <hmukos@...dex-team.ru>
Subject: Re: [net-next] net: core: use shared sysctl macro

On Wed, Feb 23, 2022 at 9:29 AM Luis Chamberlain <mcgrof@...nel.org> wrote:
>
> On Tue, Feb 22, 2022 at 08:56:28PM +0800, xiangxia.m.yue@...il.com wrote:
> > diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> > index 6353d6db69b2..b2ac6542455f 100644
> > --- a/include/linux/sysctl.h
> > +++ b/include/linux/sysctl.h
> > @@ -42,12 +42,13 @@ struct ctl_dir;
> >  #define SYSCTL_ZERO                  ((void *)&sysctl_vals[1])
> >  #define SYSCTL_ONE                   ((void *)&sysctl_vals[2])
> >  #define SYSCTL_TWO                   ((void *)&sysctl_vals[3])
> > -#define SYSCTL_FOUR                  ((void *)&sysctl_vals[4])
> > -#define SYSCTL_ONE_HUNDRED           ((void *)&sysctl_vals[5])
> > -#define SYSCTL_TWO_HUNDRED           ((void *)&sysctl_vals[6])
> > -#define SYSCTL_ONE_THOUSAND          ((void *)&sysctl_vals[7])
> > -#define SYSCTL_THREE_THOUSAND                ((void *)&sysctl_vals[8])
> > -#define SYSCTL_INT_MAX                       ((void *)&sysctl_vals[9])
> > +#define SYSCTL_THREE                 ((void *)&sysctl_vals[4])
> > +#define SYSCTL_FOUR                  ((void *)&sysctl_vals[5])
> > +#define SYSCTL_ONE_HUNDRED           ((void *)&sysctl_vals[6])
> > +#define SYSCTL_TWO_HUNDRED           ((void *)&sysctl_vals[7])
> > +#define SYSCTL_ONE_THOUSAND          ((void *)&sysctl_vals[8])
> > +#define SYSCTL_THREE_THOUSAND                ((void *)&sysctl_vals[9])
> > +#define SYSCTL_INT_MAX                       ((void *)&sysctl_vals[10])
>
> xiangxia, thanks for you patch!
>
> I welcome this change but can you please also extend lib/test_sysctl.c
> (selftests) and/or kernel/sysctl-test.c (UML kunit test) to ensure we
> don't regress any existing mappings here.
>
> The test can be really simply and would seem stupid but it would be of
> great help. It would just make sure SYSCTL_ONE == 1, SYSCTL_TWO == 2, etc.
>
> I think using kunit makes more sense here. Once you then then have this
> test, you can use it to verify you have not introduced a regression and
> re-send the patch.
Sorry for taking so long to reply.

KUnit:
[23:03:58] ================ sysctl_test (10 subtests) =================
[23:03:58] [PASSED] sysctl_test_api_dointvec_null_tbl_data
[23:03:58] [PASSED] sysctl_test_api_dointvec_table_maxlen_unset
[23:03:58] [PASSED] sysctl_test_api_dointvec_table_len_is_zero
[23:03:58] [PASSED] sysctl_test_api_dointvec_table_read_but_position_set
[23:03:58] [PASSED] sysctl_test_dointvec_read_happy_single_positive
[23:03:58] [PASSED] sysctl_test_dointvec_read_happy_single_negative
[23:03:58] [PASSED] sysctl_test_dointvec_write_happy_single_positive
[23:03:58] [PASSED] sysctl_test_dointvec_write_happy_single_negative
[23:03:58] [PASSED] sysctl_test_api_dointvec_write_single_less_int_min
[23:03:58] [PASSED] sysctl_test_api_dointvec_write_single_greater_int_max
[23:03:58] =================== [PASSED] sysctl_test ===================


./run_kselftest.sh -c sysctl
....
# Running test: sysctl_test_0006 - run #49
# Checking bitmap handler... ok
# Wed Mar 16 14:58:41 UTC 2022
# Running test: sysctl_test_0007 - run #0
# Boot param test only possible sysctl_test is built-in, not module:
# CONFIG_TEST_SYSCTL=m
ok 1 selftests: sysctl: sysctl.sh


> Thanks!
>
>   Luis



-- 
Best regards, Tonghao