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;
MIME-Version: 1.0
References: <20200403171303.GK20760@hirez.programming.kicks-ass.net>
 <20200808101222.5103093e@coco.lan> <20200810092523.GA8612@linux-8ccs>
 <20200810150647.GB8612@linux-8ccs> <20200811163427.6edbf343@coco.lan>
 <20200811145524.GE2674@hirez.programming.kicks-ass.net> <20200811172738.2d632a09@coco.lan>
 <20200811160134.GA13652@linux-8ccs> <CAMj1kXF8fm=9CdQykqDbgYCJSP88ezMs3EOosCW+SDi+Lve0zg@mail.gmail.com>
 <20200812104005.GN2674@hirez.programming.kicks-ass.net> <20200812114127.GA10824@linux-8ccs.fritz.box>
In-Reply-To: <20200812114127.GA10824@linux-8ccs.fritz.box>
From:   "H.J. Lu" <hjl.tools@gmail.com>
Date:   Wed, 12 Aug 2020 06:14:08 -0700
Message-ID: <CAMe9rOqVbZ=TmEtPXVU_rQbvNkmzht2bxs5qWuFzS3GLSbEOLQ@mail.gmail.com>
Subject: Re: [PATCH v2] module: Harden STRICT_MODULE_RWX
To:     Jessica Yu <jeyu@kernel.org>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Kees Cook <keescook@chromium.org>,
        Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Binutils <binutils@sourceware.org>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Miroslav Benes <mbenes@suse.cz>,
        Ard Biesheuvel <ardb@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Aug 12, 2020 at 4:42 AM Jessica Yu via Binutils
<binutils@sourceware.org> wrote:
>
> +++ peterz@infradead.org [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
> >elsewhere.
>
> 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 module.lds a bit and noticed that the
> name of the section seems to matters. So this:
>
>   SECTIONS {
>       .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.
>

Please open a binutils bug with a testcase.


-- 
H.J.