Re: [PATCH v2] module: Harden STRICT_MODULE_RWX

From: Mauro Carvalho Chehab
Date: Tue Aug 11 2020 - 10:34:34 EST

Next message: Turritopsis Dohrnii Teo En Ming: "How to Setup TWO DMZ Zones (DMZ1 and DMZ2) on the Cisco ASA 5506-X Firewall"
Previous message: Andi Kleen: "Re: [PATCH 1/2] perf: Add closing sibling events' file descriptors"
In reply to: Jessica Yu: "Re: [PATCH v2] module: Harden STRICT_MODULE_RWX"
Next in thread: peterz: "Re: [PATCH v2] module: Harden STRICT_MODULE_RWX"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Jessica,

Em Mon, 10 Aug 2020 17:06:50 +0200
Jessica Yu <jeyu@xxxxxxxxxx> escreveu:

> +++ Jessica Yu [10/08/20 11:25 +0200]:
> >+++ Mauro Carvalho Chehab [08/08/20 10:12 +0200]:
> >[snip]
> >>Right now, what happens is:
> >>
> >> # modprobe wlcore
> >> modprobe: ERROR: could not insert 'wlcore': Exec format error
> >>
> >>This seems to be failing for all modules, as doesn't show anything
> >>probed.
> >>
> >>Btw, IMO, it would be useful to have some pr_debug() infra in order to
> >>explain why insmod is failing, or to have more error codes used there,
> >>as nothing was printed at dmesg. That makes harder to debug issues
> >>there. I ended losing a lot of time yesterday rebuilding the Kernel
> >>and checking the FS, before deciding to add some printks inside the
> >>Kernel ;-)
> >>
> >>In order for modprobe to start working again, I had to apply this
> >>dirty hack:
> >>
> >><hack>
> >>diff --git a/kernel/module.c b/kernel/module.c
> >>index 910a57640818..10d590dc48ad 100644
> >>--- a/kernel/module.c
> >>+++ b/kernel/module.c
> >>@@ -2051,11 +2051,12 @@ static int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
> >> const unsigned long shf_wx = SHF_WRITE|SHF_EXECINSTR;
> >> int i;
> >>
> >>+#if 0
> >> for (i = 0; i < hdr->e_shnum; i++) {
> >> if ((sechdrs[i].sh_flags & shf_wx) == shf_wx)
> >> return -ENOEXEC;
> >> }
> >>-
> >>+#endif
> >> return 0;
> >>}
> >></hack>
>
> [ I somehow munged the To: header in the last mail. Sorry about that,
> it's fixed now. ]
>
> >All this hunk does it reject loading modules that have any sections
> >that have both the writable and executable flags. You're saying it's
> >happening for all modules on your setup - I am curious as to which
> >sections have both these flags

Yes, without the hack, all modules are rejected.

> what does readelf -S tell you?

<snip>
$ readelf -S ./drivers/net/wireless/ti/wlcore/wlcore.ko
There are 54 section headers, starting at offset 0x8acd08:

Section Headers:
[Nr] Name Type Address Offset
Size EntSize Flags Link Info Align
[ 0] NULL 0000000000000000 00000000
0000000000000000 0000000000000000 0 0 0
[ 1] .text PROGBITS 0000000000000000 00000040
000000000001ddb8 0000000000000000 AX 0 0 8
[ 2] .rela.text RELA 0000000000000000 004c3db8
000000000001b648 0000000000000018 I 51 1 8
[ 3] .text.unlikely PROGBITS 0000000000000000 0001ddf8
0000000000000388 0000000000000000 AX 0 0 4
[ 4] .rela.text.u[...] RELA 0000000000000000 004df400
00000000000003f0 0000000000000018 I 51 3 8
[ 5] __ksymtab PROGBITS 0000000000000000 0001e180
0000000000000030 0000000000000000 A 0 0 4
[ 6] .rela__ksymtab RELA 0000000000000000 004df7f0
0000000000000120 0000000000000018 I 51 5 8
[ 7] __ksymtab_gpl PROGBITS 0000000000000000 0001e1b0
0000000000000228 0000000000000000 A 0 0 4
[ 8] .rela__ksymt[...] RELA 0000000000000000 004df910
0000000000000cf0 0000000000000018 I 51 7 8
[ 9] .rodata PROGBITS 0000000000000000 0001e3d8
000000000000309d 0000000000000000 A 0 0 8
[10] .rela.rodata RELA 0000000000000000 004e0600
0000000000000d08 0000000000000018 I 51 9 8
[11] __ksymtab_strings PROGBITS 0000000000000000 00021475
00000000000004eb 0000000000000001 AMS 0 0 1
[12] .rodata.str PROGBITS 0000000000000000 00021960
000000000000160d 0000000000000001 AMS 0 0 1
[13] .rodata.str1.8 PROGBITS 0000000000000000 00022f70
0000000000006724 0000000000000001 AMS 0 0 8
[14] .modinfo PROGBITS 0000000000000000 00029694
0000000000000238 0000000000000000 A 0 0 1
[15] __param PROGBITS 0000000000000000 000298d0
00000000000000c8 0000000000000000 A 0 0 8
[16] .rela__param RELA 0000000000000000 004e1308
00000000000001e0 0000000000000018 I 51 15 8
[17] .altinstructions PROGBITS 0000000000000000 00029998
0000000000000018 0000000000000000 A 0 0 1
[18] .rela.altins[...] RELA 0000000000000000 004e14e8
0000000000000060 0000000000000018 I 51 17 8
[19] .eh_frame PROGBITS 0000000000000000 000299b0
000000000000542c 0000000000000000 A 0 0 8
[20] .rela.eh_frame RELA 0000000000000000 004e1548
0000000000002040 0000000000000018 I 51 19 8
[21] .note.gnu.pr[...] NOTE 0000000000000000 0002ede0
0000000000000020 0000000000000000 A 0 0 8
[22] .note.gnu.bu[...] NOTE 0000000000000000 0002ee00
0000000000000024 0000000000000000 A 0 0 4
[23] .note.Linux NOTE 0000000000000000 0002ee24
0000000000000018 0000000000000000 A 0 0 4
[24] __jump_table PROGBITS 0000000000000000 0002ee40
0000000000002370 0000000000000000 WA 0 0 8
[25] .rela__jump_table RELA 0000000000000000 004e3588
0000000000009f78 0000000000000018 I 51 24 8
[26] .data PROGBITS 0000000000000000 000311b0
0000000000000d30 0000000000000000 WA 0 0 8
[27] .rela.data RELA 0000000000000000 004ed500
00000000000000d8 0000000000000018 I 51 26 8
[28] __bug_table PROGBITS 0000000000000000 00031ee0
0000000000000708 0000000000000000 WA 0 0 4
[29] .rela__bug_table RELA 0000000000000000 004ed5d8
0000000000001c20 0000000000000018 I 51 28 8
[30] __verbose PROGBITS 0000000000000000 000325e8
0000000000003330 0000000000000000 WA 0 0 8
[31] .rela__verbose RELA 0000000000000000 004ef1f8
00000000000057c0 0000000000000018 I 51 30 8
[32] .gnu.linkonc[...] PROGBITS 0000000000000000 00035940
0000000000000340 0000000000000000 WA 0 0 64
[33] .plt PROGBITS 0000000000000340 00035c80
0000000000000001 0000000000000000 WAX 0 0 1
[34] .init.plt NOBITS 0000000000000341 00035c81
0000000000000001 0000000000000000 WA 0 0 1
[35] .text.ftrace[...] PROGBITS 0000000000000342 00035c81
0000000000000001 0000000000000000 WAX 0 0 1
[36] .bss NOBITS 0000000000000000 00035c88
0000000000000010 0000000000000000 WA 0 0 8
[37] .comment PROGBITS 0000000000000000 00035c88
00000000000001c2 0000000000000001 MS 0 0 1
[38] .note.GNU-stack PROGBITS 0000000000000000 00035e4a
0000000000000000 0000000000000000 0 0 1
[39] .debug_aranges PROGBITS 0000000000000000 00035e4a
0000000000000310 0000000000000000 0 0 1
[40] .rela.debug_[...] RELA 0000000000000000 004f49b8
0000000000000330 0000000000000018 I 51 39 8
[41] .debug_info PROGBITS 0000000000000000 0003615a
000000000026ceba 0000000000000000 0 0 1
[42] .rela.debug_info RELA 0000000000000000 004f4ce8
00000000003695b8 0000000000000018 I 51 41 8
[43] .debug_abbrev PROGBITS 0000000000000000 002a3014
000000000000cd2c 0000000000000000 0 0 1
[44] .debug_line PROGBITS 0000000000000000 002afd40
000000000003790b 0000000000000000 0 0 1
[45] .rela.debug_line RELA 0000000000000000 0085e2a0
00000000000001f8 0000000000000018 I 51 44 8
[46] .debug_str PROGBITS 0000000000000000 002e764b
000000000017b06a 0000000000000001 MS 0 0 1
[47] .debug_loc PROGBITS 0000000000000000 004626b5
000000000002af32 0000000000000000 0 0 1
[48] .rela.debug_loc RELA 0000000000000000 0085e498
0000000000022050 0000000000000018 I 51 47 8
[49] .debug_ranges PROGBITS 0000000000000000 0048d5e7
0000000000027270 0000000000000000 0 0 1
[50] .rela.debug_[...] RELA 0000000000000000 008804e8
000000000002c610 0000000000000018 I 51 49 8
[51] .symtab SYMTAB 0000000000000000 004b4858
0000000000008ce8 0000000000000018 52 1144 8
[52] .strtab STRTAB 0000000000000000 004bd540
0000000000006872 0000000000000000 0 0 1
[53] .shstrtab STRTAB 0000000000000000 008acaf8
000000000000020c 0000000000000000 0 0 1
Key to Flags:
W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
L (link order), O (extra OS processing required), G (group), T (TLS),
C (compressed), x (unknown), o (OS specific), E (exclude),
p (processor specific)
</snip>

Let me know if the above output of readelf helps. On a quick look,
it sounded that only text and plt sections have exec flag.

> Hmm, I was not able to reproduce this with a cross-compiled kernel
> using the attached config (gcc 9.3.0 with vanilla v5.8 kernel).

That started to happen after an upgrade on Debian Bullseye, although
about the same time I also re-based my series on the top of v5.8.

So, it might be related to the upgrade or due to some differences at
the toolchain.

Btw, I'm building it natively on an arm64 machine.

Gcc is:

$ gcc --version
gcc (Debian 9.3.0-15) 9.3.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

ld is:

$ ld --version
GNU ld (GNU Binutils for Debian) 2.35
Copyright (C) 2020 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) a later version.
This program has absolutely no warranty.

The relevant Debian packages that I'm using are:

ii gcc-9 9.3.0-15 arm64 GNU C compiler
ii gcc-9-base:arm64 9.3.0-15 arm64 GCC, the GNU Compiler Collection (base package)
ii libgcc-9-dev:arm64 9.3.0-15 arm64 GCC support library (development files)
ii binutils 2.35-1 arm64 GNU assembler, linker and binary utilities
ii binutils-aarch64-linux-gnu 2.35-1 arm64 GNU binary utilities, for aarch64-linux-gnu target
ii binutils-common:arm64 2.35-1 arm64 Common files for the GNU assembler, linker and binary utilities
ii libbinutils:arm64 2.35-1 arm64 GNU binary utilities (private shared library)

> I am
> curious if the failing sections are also SHF_ALLOC - in that case, the
> code is doing what it is intended to do, which is rejecting loading
> any modules with writable and executable sections. If the problematic
> sections are *not* SHF_ALLOC, then we might be able to work around
> that by ignoring non-SHF_ALLOC sections as they are not copied to the
> final module location anyway.

Please let me know if you need any other tests from my side.

Thanks,
Mauro

Next message: Turritopsis Dohrnii Teo En Ming: "How to Setup TWO DMZ Zones (DMZ1 and DMZ2) on the Cisco ASA 5506-X Firewall"
Previous message: Andi Kleen: "Re: [PATCH 1/2] perf: Add closing sibling events' file descriptors"
In reply to: Jessica Yu: "Re: [PATCH v2] module: Harden STRICT_MODULE_RWX"
Next in thread: peterz: "Re: [PATCH v2] module: Harden STRICT_MODULE_RWX"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]