Re: [PATCH 2/2] meson: mitigate against use of uninitialize stack for exploits

[Daniel P . Berrangé]

On Mon, Oct 09, 2023 at 12:15:17PM +0200, Thomas Huth wrote:
> On 09/10/2023 09.44, Thomas Huth wrote:
> > On 05/10/2023 19.38, Daniel P. Berrangé wrote:
> > > When variables are used without being initialized, there is potential
> > > to take advantage of data that was pre-existing on the stack from an
> > > earlier call, to drive an exploit.
> > > 
> > > It is good practice to always initialize variables, and the compiler
> > > can warn about flaws when -Wuninitialized is present. This warning,
> > > however, is by no means foolproof with its output varying depending
> > > on compiler version and which optimizations are enabled.
> > > 
> > > The -ftrivial-auto-var-init option can be used to tell the compiler
> > > to always initialize all variables. This increases the security and
> > > predictability of the program, closing off certain attack vectors,
> > > reducing the risk of unsafe memory disclosure.
> > > 
> > > While the option takes several possible values, using 'zero' is
> > > considered to be the  option that is likely to lead to semantically
> > > correct or safe behaviour[1]. eg sizes/indexes are not likely to
> > > lead to out-of-bounds accesses when initialized to zero. Pointers
> > > are less likely to point something useful if initialized to zero.
> > > 
> > > Even with -ftrivial-auto-var-init=zero set, GCC will still issue
> ...
> > > +    '-ftrivial-var-auto-init=zero',
> > >   ]
> 
> There is something fishy here: In the commit description, you write about
> "-ftrivial-auto-var-init" but in the code you use "-ftrivial-var-auto-init"
> ... that looks wrong to me, please fix!

Face palm, -ftrivial-auto-var-init is the correct one.

> > I was a little bit torn about using =zero when I first read your patch,
> > but after looking at [1], I tend now also tend to agree that =zero is
> > likely the best choice.
> 
> Thinking about this twice: What about using -ftrivial-var-auto-init=pattern
> for --enable-debug builds, and only use the "zero" init for non-debug
> builds? ... that would prevent that people blindly rely on this "language
> extension".

We can't blindly rely on it, because -Wuninitialized is still going to
do static analysis and warn in most cases, which can't be ignored when
-Werror is set.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|