On Mon, Aug 15, 2016 at 03:12:53PM -0400, Jessica Yu wrote:
> +++ Jessica Yu [10/08/16 18:58 -0400]:
> >+++ Eryu Guan [10/08/16 23:21 +0800]:
> >>Hi,
> >>
> >>I hit boot failure on s390x host starting from 4.8-rc1 kernel, 4.7
> >>kernel works fine. And I bisected to this commit 444d13ff10fb
> >>
> >> commit 444d13ff10fb13bc3e64859c3cf9ce43dcfeb075
> >> Author: Jessica Yu <jeyu@xxxxxxxxxx>
> >> Date: Wed Jul 27 12:06:21 2016 +0930
> >>
> >> modules: add ro_after_init support
> >>
> >> Add ro_after_init support for modules by adding a new page-aligned section
> >> in the module layout (after rodata) for ro_after_init data and enabling RO
> >> protection for that section after module init runs.
> >>
> >> Signed-off-by: Jessica Yu <jeyu@xxxxxxxxxx>
> >> Acked-by: Kees Cook <keescook@xxxxxxxxxxxx>
> >> Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
> >>
> >>and I've only hit this panic on s390x hosts. Console log is appended at
> >>the end of email.
> >>
> >>Thanks,
> >>Eryu
> >
> >Hi Eryu, thanks for reporting this. It's a bit difficult to tell from
> >the stacktrace alone what's really going on, so I'll attempt to
> >reproduce this on a 4.8-rc1 kernel once I get my hands on an s390x
> >system and report back.
>
> [ CC'ing Heiko and Martin ]
>
> So this panic is related to some recent changes to set_memory_{ro,rw}
> on s390x, see commit e8a97e42 "s390/pageattr: allow kernel page table
> splitting." The new implementation of set_memory_{ro,rw} on s390 isn't
> handling the case when numpages is 0.
>
> Recall the general layout of a module:
> [text] [rodata] [ro-after-init] [writable data]
>
> Normally a module's ro after init section sits between rodata and
> writable data. When a module doesn't have a ro after init section,
> set_memory_ro gets called with the first page-aligned addr after
> rodata, but with numpages = 0. However in this case since
> set_memory_ro isn't handling the case when numpages is 0, it
> incorrectly ends up walking the page table anyway and ends up setting
> a normally writable page to ro. Adding a simple numpages == 0 check
> to set_memory_{ro,rw} and returning fixes the panic.
>
> Jessica
All what you write is correct. The patch below is sitting in our "fixes"
branch since a week:
https://git.kernel.org/cgit/linux/kernel/git/s390/linux.git/log/?h=fixes
I assume there will be a pull request from Martin soon.
>From 4d81aaa53c2dea220ddf88e19c33033d6cf4f8cb Mon Sep 17 00:00:00 2001
From: Heiko Carstens <heiko.carstens@xxxxxxxxxx>
Date: Tue, 9 Aug 2016 12:26:28 +0200
Subject: [PATCH] s390/pageattr: handle numpages parameter correctly
Both set_memory_ro() and set_memory_rw() will modify the page
attributes of at least one page, even if the numpages parameter is
zero.
The author expected that calling these functions with numpages == zero
would never happen. However with the new 444d13ff10fb ("modules: add
ro_after_init support") feature this happens frequently.
Therefore do the right thing and make these two functions return
gracefully if nothing should be done.
Fixes crashes on module load like this one:
Unable to handle kernel pointer dereference in virtual kernel address space
Failing address: 000003ff80008000 TEID: 000003ff80008407
Fault in home space mode while using kernel ASCE.
AS:0000000000d18007 R3:00000001e6aa4007 S:00000001e6a10800 P:00000001e34ee21d
Oops: 0004 ilc:3 [#1] SMP
Modules linked in: x_tables
CPU: 10 PID: 1 Comm: systemd Not tainted 4.7.0-11895-g3fa9045 #4
Hardware name: IBM 2964 N96 703 (LPAR)
task: 00000001e9118000 task.stack: 00000001e9120000
Krnl PSW : 0704e00180000000 00000000005677f8 (rb_erase+0xf0/0x4d0)
R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
Krnl GPRS: 000003ff80008b20 000003ff80008b20 000003ff80008b70 0000000000b9d608
000003ff80008b20 0000000000000000 00000001e9123e88 000003ff80008950
00000001e485ab40 000003ff00000000 000003ff80008b00 00000001e4858480
0000000100000000 000003ff80008b68 00000000001d5998 00000001e9123c28
Krnl Code: 00000000005677e8: ec1801c3007c cgij %r1,0,8,567b6e
00000000005677ee: e32010100020 cg %r2,16(%r1)
#00000000005677f4: a78401c2 brc 8,567b78
>00000000005677f8: e35010080024 stg %r5,8(%r1)
00000000005677fe: ec5801af007c cgij %r5,0,8,567b5c
0000000000567804: e30050000024 stg %r0,0(%r5)
000000000056780a: ebacf0680004 lmg %r10,%r12,104(%r15)
0000000000567810: 07fe bcr 15,%r14
Call Trace:
([<000003ff80008900>] __this_module+0x0/0xffffffffffffd700 [x_tables])
([<0000000000264fd4>] do_init_module+0x12c/0x220)
([<00000000001da14a>] load_module+0x24e2/0x2b10)
([<00000000001da976>] SyS_finit_module+0xbe/0xd8)
([<0000000000803b26>] system_call+0xd6/0x264)
Last Breaking-Event-Address:
[<000000000056771a>] rb_erase+0x12/0x4d0
Kernel panic - not syncing: Fatal exception: panic_on_oops
Reported-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
Reported-and-tested-by: Sebastian Ott <sebott@xxxxxxxxxxxxxxxxxx>
Fixes: e8a97e42dc98 ("s390/pageattr: allow kernel page table splitting")
Signed-off-by: Heiko Carstens <heiko.carstens@xxxxxxxxxx>
Signed-off-by: Martin Schwidefsky <schwidefsky@xxxxxxxxxx>
---
arch/s390/mm/pageattr.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/s390/mm/pageattr.c b/arch/s390/mm/pageattr.c
index 7104ffb5a67f..af7cf28cf97e 100644
--- a/arch/s390/mm/pageattr.c
+++ b/arch/s390/mm/pageattr.c
@@ -252,6 +252,8 @@ static int change_page_attr(unsigned long addr, unsigned long end,
int rc = -EINVAL;
pgd_t *pgdp;
+ if (addr == end)
+ return 0;
if (end >= MODULES_END)
return -EINVAL;
mutex_lock(&cpa_mutex);
--
2.6.6
--
To unsubscribe from this list: send the line "unsubscribe live-patching" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
