diff mbox series

[v2,2/2] meson: mitigate against use of uninitialize stack for exploits

Message ID 20240103123414.2401208-3-berrange@redhat.com
State New
Headers show
Series topic: meson: add more compiler hardening flags | expand

Commit Message

Daniel P. Berrangé Jan. 3, 2024, 12:34 p.m. UTC
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
warnings with -Wuninitialized if it discovers a problem, so we are
not loosing diagnostics for developers, just hardening runtime
behaviour and making QEMU behave more predictably in case of hitting
bad codepaths.

[1] https://lists.llvm.org/pipermail/cfe-dev/2020-April/065221.html
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 meson.build | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Markus Armbruster Jan. 9, 2024, 2:48 p.m. UTC | #1
Daniel P. Berrangé <berrange@redhat.com> writes:

> 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
> warnings with -Wuninitialized if it discovers a problem, so we are
> not loosing diagnostics for developers, just hardening runtime
> behaviour and making QEMU behave more predictably in case of hitting
> bad codepaths.
>
> [1] https://lists.llvm.org/pipermail/cfe-dev/2020-April/065221.html
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  meson.build | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index eaa20d241d..efc1b4dd14 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -440,6 +440,11 @@ hardening_flags = [
>      # upon its return. This makes it harder to assemble
>      # ROP gadgets into something usable
>      '-fzero-call-used-regs=used-gpr',
> +
> +    # Initialize all stack variables to zero. This makes
> +    # it harder to take advantage of uninitialized stack
> +    # data to drive exploits
> +    '-ftrivial-auto-var-init=zero',
>  ]
>  
>  qemu_common_flags += cc.get_supported_arguments(hardening_flags)

Have you tried to throw in -Wtrivial-auto-var-init?

Documentation, for your convenience:

‘-Wtrivial-auto-var-init’
     Warn when ‘-ftrivial-auto-var-init’ cannot initialize the automatic
     variable.  A common situation is an automatic variable that is
     declared between the controlling expression and the first case
     label of a ‘switch’ statement.
Daniel P. Berrangé Jan. 9, 2024, 2:53 p.m. UTC | #2
On Tue, Jan 09, 2024 at 03:48:42PM +0100, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > 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
> > warnings with -Wuninitialized if it discovers a problem, so we are
> > not loosing diagnostics for developers, just hardening runtime
> > behaviour and making QEMU behave more predictably in case of hitting
> > bad codepaths.
> >
> > [1] https://lists.llvm.org/pipermail/cfe-dev/2020-April/065221.html
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  meson.build | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/meson.build b/meson.build
> > index eaa20d241d..efc1b4dd14 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -440,6 +440,11 @@ hardening_flags = [
> >      # upon its return. This makes it harder to assemble
> >      # ROP gadgets into something usable
> >      '-fzero-call-used-regs=used-gpr',
> > +
> > +    # Initialize all stack variables to zero. This makes
> > +    # it harder to take advantage of uninitialized stack
> > +    # data to drive exploits
> > +    '-ftrivial-auto-var-init=zero',
> >  ]
> >  
> >  qemu_common_flags += cc.get_supported_arguments(hardening_flags)
> 
> Have you tried to throw in -Wtrivial-auto-var-init?
> 
> Documentation, for your convenience:
> 
> ‘-Wtrivial-auto-var-init’
>      Warn when ‘-ftrivial-auto-var-init’ cannot initialize the automatic
>      variable.  A common situation is an automatic variable that is
>      declared between the controlling expression and the first case
>      label of a ‘switch’ statement.

No, I didn't notice that warning.  I'll have a look if it reoprts
any problems, but not optimistic since we probably have such code
patterns.

With regards,
Daniel
diff mbox series

Patch

diff --git a/meson.build b/meson.build
index eaa20d241d..efc1b4dd14 100644
--- a/meson.build
+++ b/meson.build
@@ -440,6 +440,11 @@  hardening_flags = [
     # upon its return. This makes it harder to assemble
     # ROP gadgets into something usable
     '-fzero-call-used-regs=used-gpr',
+
+    # Initialize all stack variables to zero. This makes
+    # it harder to take advantage of uninitialized stack
+    # data to drive exploits
+    '-ftrivial-auto-var-init=zero',
 ]
 
 qemu_common_flags += cc.get_supported_arguments(hardening_flags)