Thread (15 messages) 15 messages, 4 authors, 2024-03-04

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

From: Kees Cook <hidden>
Date: 2024-02-28 17:21:34
Also in: linux-alpha, linux-arm-kernel, linux-mips, linux-mm, linux-s390, linux-sh, lkml, loongarch, sparclinux

On Wed, Feb 28, 2024 at 01:22:09PM +0000, Christophe Leroy wrote:
[...]
My worry with initialisation at declaration is it often hides missing 
assignments. Let's take following simple exemple:

char *colour(int num)
{
	char *name;

	if (num == 0) {
		name = "black";
	} else if (num == 1) {
		name = "white";
	} else if (num == 2) {
	} else {
		name = "no colour";
	}

	return name;
}

Here, GCC warns about a missing initialisation of variable 'name'.
Sometimes. :( We build with -Wno-maybe-uninitialized because GCC gets
this wrong too often. Also, like with large structs like this, all
uninit warnings get suppressed if anything takes it by reference. So, if
before your "return name" statement above, you had something like:

	do_something(&name);

it won't warn with any option enabled.
But if I declare it as

	char *name = "no colour";

Then GCC won't warn anymore that we are missing a value for when num is 2.

During my life I have so many times spent huge amount of time 
investigating issues and bugs due to missing assignments that were going 
undetected due to default initialisation at declaration.
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.

-Kees

-- 
Kees Cook
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help