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

Message-ID: <YhWOETp0UB9IpU6R@bombadil.infradead.org>
Date:   Tue, 22 Feb 2022 17:29:53 -0800
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     xiangxia.m.yue@...il.com
Cc:     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 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.

Thanks!

  Luis