Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 25 Jan 2021 15:27:55 -0600
From:   Josh Poimboeuf <jpoimboe@redhat.com>
To:     Masahiro Yamada <masahiroy@kernel.org>
Cc:     Kees Cook <keescook@chromium.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Michal Marek <michal.lkml@markovi.net>,
        linux-hardening@vger.kernel.org,
        Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Justin Forbes <jforbes@redhat.com>,
        Ondrej Mosnacek <omosnace@redhat.com>
Subject: Re: [PATCH RFC] gcc-plugins: Handle GCC version mismatch for OOT
 modules
Message-ID: <20210125212755.jfwlqogpcarmxdgt@treble>
References: <efe6b039a544da8215d5e54aa7c4b6d1986fc2b0.1611607264.git.jpoimboe@redhat.com>
 <CAK7LNAS=uOi=8xJU=NiKnXQW2iCazbErg_TX0gL9oayBiDffiA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CAK7LNAS=uOi=8xJU=NiKnXQW2iCazbErg_TX0gL9oayBiDffiA@mail.gmail.com>
Precedence: bulk

On Tue, Jan 26, 2021 at 06:16:01AM +0900, Masahiro Yamada wrote:
> On Tue, Jan 26, 2021 at 5:42 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >
> > When building out-of-tree kernel modules, the build system doesn't
> > require the GCC version to match the version used to build the original
> > kernel.  That's probably [1] fine.
> >
> > In fact, for many distros, the version of GCC used to build the latest
> > kernel doesn't necessarily match the latest released GCC, so a GCC
> > mismatch turns out to be pretty common.  And with CONFIG_MODVERSIONS
> > it's probably more common.
> >
> > So a lot of users have come to rely on being able to use a different
> > version of GCC when building OOT modules.
> >
> > But with GCC plugins enabled, that's no longer allowed:
> >
> >   cc1: error: incompatible gcc/plugin versions
> >   cc1: error: failed to initialize plugin ./scripts/gcc-plugins/structleak_plugin.so
> >
> > That error comes from the plugin's call to
> > plugin_default_version_check(), which strictly enforces the GCC version.
> > The strict check makes sense, because there's nothing to prevent the GCC
> > plugin ABI from changing -- and it often does.
> >
> > But failing the build isn't necessary.  For most plugins, OOT modules
> > will otherwise work just fine without the plugin instrumentation.
> >
> > When a GCC version mismatch is detected, print a warning and disable the
> > plugin.  The only exception is the RANDSTRUCT plugin which needs all
> > code to see the same struct layouts.  In that case print an error.
> >
> > [1] Ignoring, for the moment, that the kernel now has
> >     toolchain-dependent kconfig options, which can silently disable
> >     features and cause havoc when compiler versions differ, or even when
> >     certain libraries are missing.  This is a separate problem which
> >     also needs to be addressed.
> >
> > Reported-by: Ondrej Mosnacek <omosnace@redhat.com>
> > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> > ---
> 
> 
> We are based on the assumption that we use the same
> compiler for in-tree and out-of-tree.

Sorry, but that assumption isn't based in reality.  And it's not
enforced.

> If people use a different compiler, they must be
> prepared for any possible problem.
>
> Using different compiler flags for in-tree and out-of-tree
> is even more dangerous.
> 
> For example, CONFIG_GCC_PLUGIN_RANDSTRUCT is enabled
> for in-tree build, and then disabled for out-of-tree modules,
> the struct layout will mismatch, won't it?

If you read the patch you'll notice that it handles that case, when it's
caused by GCC mismatch.

However, as alluded to in the [1] footnote, it doesn't handle the case
where the OOT build system doesn't have gcc-plugin-devel installed.
Then CONFIG_GCC_PLUGIN_RANDSTRUCT gets silently disabled and the build
succeeds!  That happens even without a GCC mismatch.

> This patch is ugly, and not doing the right thing.

Maybe, but I doubt the solution is to make assumptions.

-- 
Josh