Message-ID: <20210322231332.GA1984184@gmail.com>
Date:   Tue, 23 Mar 2021 00:13:32 +0100
From:   Ingo Molnar <mingo@...nel.org>
To:     Martin Sebor <msebor@...il.com>
Cc:     Arnd Bergmann <arnd@...nel.org>, linux-kernel@...r.kernel.org,
        Martin Sebor <msebor@....gnu.org>,
        Ning Sun <ning.sun@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        x86@...nel.org, Arnd Bergmann <arnd@...db.de>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Kalle Valo <kvalo@...eaurora.org>,
        Simon Kelley <simon@...kelleys.org.uk>,
        James Smart <james.smart@...adcom.com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        Anders Larsen <al@...rsen.net>, Tejun Heo <tj@...nel.org>,
        Serge Hallyn <serge@...lyn.com>,
        Imre Deak <imre.deak@...el.com>,
        linux-arm-kernel@...ts.infradead.org,
        tboot-devel@...ts.sourceforge.net, intel-gfx@...ts.freedesktop.org,
        dri-devel@...ts.freedesktop.org, ath11k@...ts.infradead.org,
        linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
        linux-scsi@...r.kernel.org, cgroups@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Lu Baolu <baolu.lu@...ux.intel.com>,
        Will Deacon <will@...nel.org>
Subject: Re: [PATCH 02/11] x86: tboot: avoid Wstringop-overread-warning


* Martin Sebor <msebor@...il.com> wrote:

> > I.e. the real workaround might be to turn off the -Wstringop-overread-warning,
> > until GCC-11 gets fixed?
> 
> In GCC 10 -Wstringop-overread is a subset of -Wstringop-overflow.
> GCC 11 breaks it out as a separate warning to make it easier to
> control.  Both warnings have caught some real bugs but they both
> have a nonzero rate of false positives.  Other than bug reports
> we don't have enough data to say what their S/N ratio might be
> but my sense is that it's fairly high in general.
> 
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=wstringop-overread
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=wstringop-overflow
> 
> In GCC 11, all access warnings expect objects to be either declared
> or allocated.  Pointers with constant values are taken to point to
> nothing valid (as Arnd mentioned above, this is to detect invalid
> accesses to members of structs at address zero).
> 
> One possible solution to the known address problem is to extend GCC
> attributes address and io that pin an object to a hardwired address
> to all targets (at the moment they're supported on just one or two
> targets).  I'm not sure this can still happen before GCC 11 releases
> sometime in April or May.
> 
> Until then, another workaround is to convert the fixed address to
> a volatile pointer before using it for the access, along the lines
> below.  It should have only a negligible effect on efficiency.

Thank you for the detailed answer!

I think I'll go with Arnd's original patch - which makes the code a 
slightly bit cleaner by separating out the check_tboot_version() check 
into a standalone function.

The only ugly aspect is the global nature of the 'tboot' pointer - but 
that's a self-inflicted wound.

I'd also guess that the S/N ratio somewhat unfairly penalizes this 
warning right now, because the kernel had a decade of growing real 
fixes via other efforts such as static and dynamic instrumentation as 
well.

So the probability of false positive remaining is in fact higher, and 
going forward we should see a better S/N ratio of this warning. Most 
of which will never be seen by upstream maintainers, as the mishaps 
will stay at the individual developer level. :-)

Thanks,

	Ingo

Powered by blists - more mailing lists