linux-kernel - Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area

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 for Android: free password hash cracker in your pocket

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

Date: Sat, 2 Mar 2024 00:47:08 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "keescook@...omium.org" <keescook@...omium.org>,
	"christophe.leroy@...roup.eu" <christophe.leroy@...roup.eu>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"x86@...nel.org" <x86@...nel.org>, "luto@...nel.org" <luto@...nel.org>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	"debug@...osinc.com" <debug@...osinc.com>, "akpm@...ux-foundation.org"
	<akpm@...ux-foundation.org>, "linux-sh@...r.kernel.org"
	<linux-sh@...r.kernel.org>, "linux-csky@...r.kernel.org"
	<linux-csky@...r.kernel.org>, "mingo@...hat.com" <mingo@...hat.com>,
	"kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
	"tglx@...utronix.de" <tglx@...utronix.de>, "linux-parisc@...r.kernel.org"
	<linux-parisc@...r.kernel.org>, "loongarch@...ts.linux.dev"
	<loongarch@...ts.linux.dev>, "linux-mm@...ck.org" <linux-mm@...ck.org>,
	"linux-snps-arc@...ts.infradead.org" <linux-snps-arc@...ts.infradead.org>,
	"Liam.Howlett@...cle.com" <Liam.Howlett@...cle.com>, "hpa@...or.com"
	<hpa@...or.com>, "peterz@...radead.org" <peterz@...radead.org>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
	"bp@...en8.de" <bp@...en8.de>, "linux-s390@...r.kernel.org"
	<linux-s390@...r.kernel.org>, "linux-alpha@...r.kernel.org"
	<linux-alpha@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-mips@...r.kernel.org"
	<linux-mips@...r.kernel.org>, "sparclinux@...r.kernel.org"
	<sparclinux@...r.kernel.org>, "broonie@...nel.org" <broonie@...nel.org>
Subject: Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

On Wed, 2024-02-28 at 09:21 -0800, Kees Cook wrote:
> I totally understand. If the "uninitialized" warnings were actually
> reliable, I would agree. I look at it this way:
> 
> - initializations can be missed either in static initializers or via
>   run time initializers. (So the risk of mistake here is matched --
>   though I'd argue it's easier to *find* static initializers when
> adding
>   new struct members.)
> - uninitialized warnings are inconsistent (this becomes an unknown
> risk)
> - when a run time initializer is missed, the contents are whatever
> was
>   on the stack (high risk)
> - what a static initializer is missed, the content is 0 (low risk)
> 
> I think unambiguous state (always 0) is significantly more important
> for
> the safety of the system as a whole. Yes, individual cases maybe bad
> ("what uid should this be? root?!") but from a general memory safety
> perspective the value doesn't become potentially influenced by order
> of
> operations, leftover stack memory, etc.
> 
> I'd agree, lifting everything into a static initializer does seem
> cleanest of all the choices.

Hi Kees,

Well, I just gave this a try. It is giving me flashbacks of when I last
had to do a tree wide change that I couldn't fully test and the
breakage was caught by Linus.

Could you let me know if you think this is additionally worthwhile
cleanup outside of the guard gap improvements of this series? Because I
was thinking a more cowardly approach could be a new vm_unmapped_area()
variant that takes the new start gap member as a separate argument
outside of struct vm_unmapped_area_info. It would be kind of strange to
keep them separate, but it would be less likely to bump something.

Thanks,

Rick