Re: [PATCH v2] module: Harden STRICT_MODULE_RWX

From: Will Deacon <will@kernel.org>
To: Szabolcs Nagy <szabolcs.nagy@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>, Jessica Yu <jeyu@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Kees Cook <keescook@chromium.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Miroslav Benes <mbenes@suse.cz>,
	Mark Rutland <mark.rutland@arm.com>, nd <nd@arm.com>
Subject: Re: [PATCH v2] module: Harden STRICT_MODULE_RWX
Date: Thu, 13 Aug 2020 10:00:06 +0100	[thread overview]
Message-ID: <20200813090006.GA9829@willie-the-truck> (raw)
In-Reply-To: <20200812164205.GS14398@arm.com>

On Wed, Aug 12, 2020 at 05:42:05PM +0100, Szabolcs Nagy wrote:
> The 08/12/2020 18:37, Ard Biesheuvel wrote:
> > On Wed, 12 Aug 2020 at 18:00, Jessica Yu <jeyu@kernel.org> wrote:
> > > +++ Szabolcs Nagy [12/08/20 15:15 +0100]:
> > > >for me it bisects to
> > > >
> > > >https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=8c803a2dd7d3d742a3d0071914f557ef465afe71
> > > >
> > > >i will have to investigate further what's going on.
> > >
> > > Thanks for the hint. I'm almost certain it's due to this excerpt from
> > > the changelog: "we now init sh_type and sh_flags for all known ABI sections
> > > in _bfd_elf_new_section_hook."
> > >
> > > Indeed, .plt and .text.* are listed as special sections in bfd/elf.c.
> > > The former requires an exact match and the latter only has to match
> > > the prefix ".text." Since the code considers ".plt" and
> > > ".text.ftrace_trampoline" special sections, their sh_type and sh_flags
> > > are now set by default. Now I guess the question is whether this can
> > > be overriden by a linker script..
> > >
> > 
> > If this is even possible to begin with, doing this in a way that is
> > portable across the various linkers that we claim to support is going
> > to be tricky.
> > 
> > I suppose this is the downside of using partially linked objects as
> > our module format - using ordinary shared libraries (along with the
> > appropriate dynamic relocations which are mostly portable across
> > architectures) would get rid of most of the PLT and trampoline issues,
> > and of a lot of complex static relocation processing code.
> > 
> > I know there is little we can do at this point, apart from ignoring
> > the permissions - perhaps we should just defer the w^x check until
> > after calling module_frob_arch_sections()?
> 
> i opened
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=26378
> 
> and waiting for binutils maintainers if this is intentional.

Thanks, Szabolcs. It's looking this is intentional behaviour (a bug fix),
so we'll have to suck it up in the module loader.

Will