Re: [PATCH v2] module: Harden STRICT_MODULE_RWX

From: Jessica Yu
Date: Wed Aug 12 2020 - 07:44:38 EST

+++ peterz@xxxxxxxxxxxxx [12/08/20 12:40 +0200]:
On Wed, Aug 12, 2020 at 10:56:56AM +0200, Ard Biesheuvel wrote:
The module .lds has BYTE(0) in the section contents to prevent the
linker from pruning them entirely. The (NOLOAD) is there to ensure
that this byte does not end up in the .ko, which is more a matter of
principle than anything else, so we can happily drop that if it helps.

However, this should only affect the PROGBITS vs NOBITS designation,
and so I am not sure whether it makes a difference.

Depending on where the w^x check occurs, we might simply override the
permissions of these sections, and strip the writable permission if it
is set in the PLT handling init code, which manipulates the metadata
of all these 3 sections before the module space is vmalloc'ed.

What's curious is that this seems the result of some recent binutils
change. Every build with binutils-2.34 (or older) does not seem to
generate these as WAX, but has the much more sensible WA.

I suppose we can change the kernel check and 'allow' W^X for 0 sized
sections, but I think we should still figure out why binutils-2.35 is
now generating WAX sections all of a sudden, it might come bite us

I have just tested with binutils-2.35 and am observing the same
behavior. Both .plt and .text.ftrace_trampoline end up with
SHT_PROGBITS and are marked 'WAX'. With binutils-2.34 they keep the
NOBITS designation.

I had thought NOLOAD implies NOBITS, but that doesn't seem to be the
case anymore? I tinkered with a bit and noticed that the
name of the section seems to matters. So this:

.plt (NOLOAD) : { BYTE(0) }
.init.plt (NOLOAD) : { BYTE(0) }
.text.ftrace_trampoline (NOLOAD) : { BYTE(0) }
+ .test (NOLOAD) : { BYTE(0) }
+ .text.test (NOLOAD) : { BYTE(0) }
+ .plt.test (NOLOAD) : { BYTE(0) }

Yielded the following:

[22] .plt PROGBITS 0000000000000340 000e40 000001 00 WAX 0 0 1
[23] .init.plt NOBITS 0000000000000341 000e41 000001 00 WA 0 0 1
[24] .text.ftrace_trampoline PROGBITS 0000000000000342 000e41 000001 00 WAX 0 0 1
[25] .test NOBITS 0000000000000343 000e42 000001 00 WA 0 0 1
[26] .text.test PROGBITS 0000000000000344 000e42 000001 00 WAX 0 0 1
[27] .plt.test NOBITS 0000000000000345 000e43 000001 00 WA 0 0 1

So ".plt" and any section starting with ".text" were automatically
designated as SHT_PROGBITS and given the executable flag. It appears
the NOLOAD directive has been ignored or overridden in the case of
these sections. I am not sure if this is a bug in binutils or if this
behavior is intentional.

I tried to grok the changelog between 2.34 and 2.35 but could not find
anything glaringly obvious that would cause this change. CC'ing the
binutils mailing list, hopefully that's the right place to ask.