linux-kernel - Re: [PATCH net-next] net: sysctl: cleanup net_sysctl

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: <20210107094146.37f20e69@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Thu, 7 Jan 2021 09:41:46 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Alexander Lobakin <alobakin@...me>
Cc:     "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next] net: sysctl: cleanup net_sysctl_init()

On Thu, 07 Jan 2021 09:13:40 +0000 Alexander Lobakin wrote:
> From: Jakub Kicinski <kuba@...nel.org>
> Date: Wed, 6 Jan 2021 16:30:56 -0800
> 
> > On Wed, 06 Jan 2021 20:40:28 +0000 Alexander Lobakin wrote:  
> >> 'net_header' is not used outside of this function, so can be moved
> >> from BSS onto the stack.
> >> Declarations of one-element arrays are discouraged, and there's no
> >> need to store 'empty' in BSS. Simply allocate it from heap at init.  
> >
> > Are you sure? It's passed as an argument to register_sysctl()
> > so it may well need to be valid for the lifetime of net_header.  
> 
> I just moved it from BSS to the heap and allocate it using kzalloc(),
> it's still valid through the lifetime of the kernel.

I see it now, please don't break the normal flow of error handling.

What's the point of moving objects allocated in __init from BSS to 
the heap? If anything I'd think it'll take up more space when allocated
in the heap because of the metadata that needs to be tracked for
dynamic allocations.

The move of net_header makes sense AFAICT, but we may have to annotate
it somehow so kmemleak doesn't complain.