LinuxLists.cc - 4.4-rc0: 5 W+X pages found

[permalink] [raw]

Subject: 4.4-rc5: ugly warn on: 5 W+X pages found

Hi!

> Kernel complains:

And now, we are at -rc5, and kernel still complains...

...with a back trace, which is clearly completely useless, and just
there to make it scary and make people report.

Problem is... noone cares for the reports. (-rc0 version below).

Can we get rid of that WARN_ON? If you do care about reports, please
add email address those can be reported to. If you don't... just drop
it.

Hardware is thinkpad x60.

Pavel

[ 3.267542] NX-protecting the kernel data: 5764k
[ 3.278325] ------------[ cut here ]------------
[ 3.282060] WARNING: CPU: 1 PID: 1 at
arch/x86/mm/dump_pagetables.c:225 note_page+0x5ec/0x790()
[ 3.285993] x86/mm: Found insecure W+X mapping at address
ffe69000/0xffe69000
[ 3.289991] Modules linked in:
[ 3.293995] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.4.0-rc5+
#132
[ 3.298178] Hardware name: LENOVO 17097HU/17097HU, BIOS 7BETD8WW
(2.19 ) 03/31/2011
[ 3.304343] 00000001 00000000 f5cffeac c42ba698 f5cffed8 f5cffec8
c404059b 000000e1
[ 3.308840] c403cb9c f5cfff50 00000163 00000000 f5cffee0 c40405f6
00000009 f5cffed8
[ 3.313419] c4d3347c f5cffef4 f5cfff1c c403cb9c c4d2c08c 000000e1
c4d3347c ffe69000
[ 3.317935] Call Trace:
[ 3.322395] [<c42ba698>] dump_stack+0x41/0x59
[ 3.326787] [<c404059b>] warn_slowpath_common+0x6b/0xa0
[ 3.331171] [<c403cb9c>] ? note_page+0x5ec/0x790
[ 3.335522] [<c40405f6>] warn_slowpath_fmt+0x26/0x30
[ 3.339839] [<c403cb9c>] note_page+0x5ec/0x790
[ 3.344154] [<c403ce9f>] ptdump_walk_pgd_level_core+0x15f/0x240
[ 3.348440] [<c403cfa1>] ptdump_walk_pgd_level_checkwx+0x11/0x20
[ 3.352611] [<c4034fdd>] mark_rodata_ro+0xcd/0xf0
[ 3.356677] [<c4a55c97>] kernel_init+0x17/0xc0
[ 3.360702] [<c4a5c409>] ret_from_kernel_thread+0x21/0x38
[ 3.364739] [<c4a55c80>] ? rest_init+0xa0/0xa0
[ 3.368709] ---[ end trace 7121849c40f4a5ba ]---
[ 3.372716] x86/mm: Checked W+X mappings: FAILED, 5 W+X pages
found.

> [ 5.256044] ------------[ cut here ]------------
> [ 5.259267] WARNING: CPU: 0 PID: 1 at
> arch/x86/mm/dump_pagetables.c:225 note_page+0x5ec/0x790()
> [ 5.262668] x86/mm: Found insecure W+X mapping at address
> ffe69000/0xffe69000
> [ 5.267109] Modules linked in:
> [ 5.271403] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.3.0+ #122
> [ 5.275679] Hardware name: LENOVO 17097HU/17097HU, BIOS 7BETD8WW
> (2.19 ) 03/31/2011
> [ 5.279957] 00000000 00000000 f5cffeac c42b9f18 f5cffed8 f5cffec8
> c404062b 000000e1
> [ 5.284387] c403ca9c f5cfff50 00000163 00000000 f5cffee0 c4040686
> 00000009 f5cffed8
> [ 5.288815] c4d268ac f5cffef4 f5cfff1c c403ca9c c4d1f494 000000e1
> c4d268ac ffe69000
> [ 5.293314] Call Trace:
> [ 5.297602] [<c42b9f18>] dump_stack+0x41/0x59
> [ 5.301864] [<c404062b>] warn_slowpath_common+0x6b/0xa0
> [ 5.306054] [<c403ca9c>] ? note_page+0x5ec/0x790
> [ 5.310209] [<c4040686>] warn_slowpath_fmt+0x26/0x30
> [ 5.314358] [<c403ca9c>] note_page+0x5ec/0x790
> [ 5.318440] [<c403cd8f>] ptdump_walk_pgd_level_core+0x14f/0x230
> [ 5.322578] [<c403ce91>] ptdump_walk_pgd_level_checkwx+0x11/0x20
> [ 5.326632] [<c4034ead>] mark_rodata_ro+0xcd/0xf0
> [ 5.330625] [<c4a4aab7>] kernel_init+0x17/0xc0
> [ 5.334585] [<c4a511c9>] ret_from_kernel_thread+0x21/0x38
> [ 5.338585] [<c4a4aaa0>] ? rest_init+0xa0/0xa0
> [ 5.342583] ---[ end trace bc9ac0874ad9a058 ]---
> [ 5.346630] x86/mm: Checked W+X mappings: FAILED, 5 W+X pages
> found.
>
> ...I'm not quite sure why it does backtrace, or how to debug this
> one...
>
> Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2015-12-14 08:58:10

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Mon, Dec 14, 2015 at 09:04:03AM +0100, Pavel Machek wrote:
> Hi!
>
> > Kernel complains:
>
> And now, we are at -rc5, and kernel still complains...

You can disable CONFIG_DEBUG_WX in your .config:

54727e6e950a ("x86: don't make DEBUG_WX default to 'y' even with DEBUG_RODATA")

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.

2015-12-14 09:07:31

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Mon 2015-12-14 09:58:03, Borislav Petkov wrote:
> On Mon, Dec 14, 2015 at 09:04:03AM +0100, Pavel Machek wrote:
> > Hi!
> >
> > > Kernel complains:
> >
> > And now, we are at -rc5, and kernel still complains...
>
> You can disable CONFIG_DEBUG_WX in your .config:
>
> 54727e6e950a ("x86: don't make DEBUG_WX default to 'y' even with DEBUG_RODATA")

I know. But either someone cares, and it should be fixes, or noone
cares, and the check should be removed.

Backtrace should be removed in any case, as it is useless.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2015-12-14 09:15:24

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Mon, Dec 14, 2015 at 10:07:26AM +0100, Pavel Machek wrote:
> I know. But either someone cares, and it should be fixes, or noone
> cares, and the check should be removed.

Someone cares.

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.

2015-12-14 12:29:55

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Mon 2015-12-14 09:04:03, Pavel Machek wrote:
> Hi!
>
> > Kernel complains:
>
> And now, we are at -rc5, and kernel still complains...
>
> ...with a back trace, which is clearly completely useless, and just
> there to make it scary and make people report.
>
> Problem is... noone cares for the reports. (-rc0 version below).
>
> Can we get rid of that WARN_ON? If you do care about reports, please
> add email address those can be reported to. If you don't... just drop
> it.
>
> Hardware is thinkpad x60.

> [ 3.285993] x86/mm: Found insecure W+X mapping at address
> ffe69000/0xffe69000

---[ Persisent kmap() Area ]---
0xffc00000-0xffd28000 1184K pte
0xffd28000-0xffddd000 724K RW GLB NX pte
0xffddd000-0xffe69000 560K pte
0xffe69000-0xffe6e000 20K RW GLB x pte
0xffe6e000-0xffe6f000 4K pte
---[ Fixmap Area ]---

Any other info that would be useful?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2015-12-14 19:18:43

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Mon, Dec 14, 2015 at 1:07 AM, Pavel Machek <[email protected]> wrote:
>
> I know. But either someone cares, and it should be fixes, or noone
> cares, and the check should be removed.

Someone cares, and it should be scheduled to be fixed for 4.5. The EFI
mapping changes that were required to avoid the warning were much too
big and late to make 4.4.

So for now, don't enable CONFIG_DEBUG_WX for now. Unless you want to
actively debug the EFI mapping changes, that is. Which I heartily
recommend people doing.

(The patches are in Matt Fleming's tree at

git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git efi-pgtable

and I think they are also merged in the -tip tree for next, but I
haven't double-checked)

Linus

2015-12-14 20:26:32

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

Hi!

> > I know. But either someone cares, and it should be fixes, or noone
> > cares, and the check should be removed.
>
> Someone cares, and it should be scheduled to be fixed for 4.5. The EFI
> mapping changes that were required to avoid the warning were much too
> big and late to make 4.4.
>
> So for now, don't enable CONFIG_DEBUG_WX for now. Unless you want to
> actively debug the EFI mapping changes, that is. Which I heartily
> recommend people doing.

Ok, good, except... This is thinkpad X60. Good old BIOS. It should
have no EFI.

pavel@duo:~$ dmesg | grep EFI
pavel@duo:~$

>From the messages I got:

> [ 3.285993] x86/mm: Found insecure W+X mapping at address
> ffe69000/0xffe69000

---[ Persisent kmap() Area ]---
0xffc00000-0xffd28000 1184K pte
0xffd28000-0xffddd000 724K RW GLB NX pte
0xffddd000-0xffe69000 560K pte
0xffe69000-0xffe6e000 20K RW GLB x pte
0xffe6e000-0xffe6f000 4K pte
---[ Fixmap Area ]---

That is not EFI, right?

Thanks,
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2015-12-14 21:02:49

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Mon, Dec 14, 2015 at 12:26 PM, Pavel Machek <[email protected]> wrote:
> Hi!
>
>> > I know. But either someone cares, and it should be fixes, or noone
>> > cares, and the check should be removed.
>>
>> Someone cares, and it should be scheduled to be fixed for 4.5. The EFI
>> mapping changes that were required to avoid the warning were much too
>> big and late to make 4.4.
>>
>> So for now, don't enable CONFIG_DEBUG_WX for now. Unless you want to
>> actively debug the EFI mapping changes, that is. Which I heartily
>> recommend people doing.
>
> Ok, good, except... This is thinkpad X60. Good old BIOS. It should
> have no EFI.
>
> pavel@duo:~$ dmesg | grep EFI
> pavel@duo:~$
>
> From the messages I got:
>
>> [ 3.285993] x86/mm: Found insecure W+X mapping at address
>> ffe69000/0xffe69000
>
> ---[ Persisent kmap() Area ]---
> 0xffc00000-0xffd28000 1184K pte
> 0xffd28000-0xffddd000 724K RW GLB NX pte
> 0xffddd000-0xffe69000 560K pte
> 0xffe69000-0xffe6e000 20K RW GLB x pte
> 0xffe6e000-0xffe6f000 4K pte
> ---[ Fixmap Area ]---
>
> That is not EFI, right?

That's weird. The only API to do that seems to be manually setting
kmap_prot to _PAGE_KERNEL_EXEC, and nothing does that. (Why is
kmap_prot a variable on x86 at all? It has exactly one writer, and
that's the code that initializes it in the first place. Shouldn't we
#define kmap_prot _PAGE_KERNEL?

--Andy

2015-12-14 21:24:39

by Arjan van de Ven

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

> That's weird. The only API to do that seems to be manually setting
> kmap_prot to _PAGE_KERNEL_EXEC, and nothing does that. (Why is
> kmap_prot a variable on x86 at all? It has exactly one writer, and
> that's the code that initializes it in the first place. Shouldn't we
> #define kmap_prot _PAGE_KERNEL?

iirc it changes based on runtime detection of NX capability

2015-12-14 22:25:33

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Mon, Dec 14, 2015 at 1:24 PM, Arjan van de Ven <[email protected]> wrote:
>
>> That's weird. The only API to do that seems to be manually setting
>> kmap_prot to _PAGE_KERNEL_EXEC, and nothing does that. (Why is
>> kmap_prot a variable on x86 at all? It has exactly one writer, and
>> that's the code that initializes it in the first place. Shouldn't we
>> #define kmap_prot _PAGE_KERNEL?
>
>
> iirc it changes based on runtime detection of NX capability
>

Maybe it did, but if it still does, I can't find the code.

What *does* change is __supported_pte_mask. If we're willing to make
disable_nx work a little less well, we could try to initialize
__supported_pte_mask from the very beginning. (We currently seem to
detect and enable NX even before we enable paging.) I suspect that
Pavel is seeing a kmap mapping left over from so early that it didn't
have NX set (killed by massage_pgprot).

Borislav, could we do that? (Why do we have disable_nx at all? I
suspect it was for debugging a long, long time ago.)

Alternatively, we could go through and set NX everywhere after we
decide we have NX, but that seems rather error-prone.

--Andy

--
Andy Lutomirski
AMA Capital Management, LLC

2015-12-15 07:57:02

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Mon 2015-12-14 13:24:08, Arjan van de Ven wrote:
>
> >That's weird. The only API to do that seems to be manually setting
> >kmap_prot to _PAGE_KERNEL_EXEC, and nothing does that. (Why is
> >kmap_prot a variable on x86 at all? It has exactly one writer, and
> >that's the code that initializes it in the first place. Shouldn't we
> >#define kmap_prot _PAGE_KERNEL?
>
> iirc it changes based on runtime detection of NX capability

Huh. Is it possible that core duo is so old that it has no NX?

processor : 1
vendor_id : GenuineIntel
cpu family : 6
model : 14
model name : Genuine Intel(R) CPU T2400 @ 1.83GHz
stepping : 8
microcode : 0x39
...
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr
pge mca cmov clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe nx
constant_tsc arch_perfmon bts aperfmperf pni monitor vmx est tm2 xtpr
pdcm dtherm

No, it lists nx in flags. Linus asked me about trying without
CONFIG_EFI. I should have no EFI here, but I'll try it.

I turned off CONFIG_EFI, but CONFIG_UEFI_CPER can't seem to be
disabled easily.

Still:

[ 3.269750] WARNING: CPU: 1 PID: 1 at
arch/x86/mm/dump_pagetables.c:225 note_page+0x5ec/0x790()
[ 3.271999] x86/mm: Found insecure W+X mapping at address
ffe69000/0xffe69000

pavel@duo:~$ zcat /proc/config.gz | grep EFI
# CONFIG_EFI_PARTITION is not set
# CONFIG_EFI is not set
CONFIG_DMI_SCAN_MACHINE_NON_EFI_FALLBACK=y
CONFIG_UEFI_CPER=y
pavel@duo:~$

Ok, I managed to turn off even CONFIG_UEFI_CPER after some fight, but
result is the same.

(Hmm... I'll probably regret it, but... I guess config.gz does contain
some information useful for the attacker. How long till some "hardened
distro" chmods it to 600?)

Best regards,

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2015-12-15 08:09:34

[permalink] [raw]

Subject: [PATCH 0/2] x86/mm: A _PAGE_NX fixlet and a kmap cleanup

The fixlet might help with some WX warnings. The kmap cleanup is
just a cleanup.

This is very lightly tested.

Andy Lutomirski (2):
x86_32/mm: Set NX in __supported_pte_mask before enabling paging
x86/mm: Make kmap_prot into a #define

arch/x86/include/asm/fixmap.h | 2 +-
arch/x86/kernel/head_32.S | 6 ++++++
arch/x86/mm/init_32.c | 3 ---
arch/x86/mm/setup_nx.c | 5 ++---
4 files changed, 9 insertions(+), 7 deletions(-)

--
2.5.0

2015-12-15 08:09:37

[permalink] [raw]

Subject: [PATCH 1/2] x86_32/mm: Set NX in __supported_pte_mask before enabling paging

There's a short window in which very early mappings can end up with
NX clear because they are created before we've noticed that we have
NX.

It turns out that we detect NX very early, so there's no need to
defer __supported_pte_mask setup.

Signed-off-by: Andy Lutomirski <[email protected]>
---
arch/x86/kernel/head_32.S | 6 ++++++
arch/x86/mm/setup_nx.c | 5 ++---
2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 6bc9ae24b6d2..57fc3f8c85fd 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -389,6 +389,12 @@ default_entry:
/* Make changes effective */
wrmsr

+ /*
+ * And make sure that all the mappings we set up have NX set from
+ * the beginning.
+ */
+ orl $(1 << (_PAGE_BIT_NX - 32)), pa(__supported_pte_mask + 4)
+
enable_paging:

/*
diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c
index 90555bf60aa4..5095ecb0719c 100644
--- a/arch/x86/mm/setup_nx.c
+++ b/arch/x86/mm/setup_nx.c
@@ -31,9 +31,8 @@ early_param("noexec", noexec_setup);

void x86_configure_nx(void)
{
- if (cpu_has_nx && !disable_nx)
- __supported_pte_mask |= _PAGE_NX;
- else
+ /* If disable_nx is set, clear NX on all new mappings going forward. */
+ if (disable_nx)
__supported_pte_mask &= ~_PAGE_NX;
}

--
2.5.0

2015-12-15 08:09:46

[permalink] [raw]

Subject: [PATCH 2/2] x86/mm: Make kmap_prot into a #define

The value (once we initialize it) is a foregone conclusion. Make it
a #define to save a tiny amount of text and data size and to make it
more comprehensible.

Signed-off-by: Andy Lutomirski <[email protected]>
---
arch/x86/include/asm/fixmap.h | 2 +-
arch/x86/mm/init_32.c | 3 ---
2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h
index 32c60a02ec80..ce941b7e7900 100644
--- a/arch/x86/include/asm/fixmap.h
+++ b/arch/x86/include/asm/fixmap.h
@@ -142,7 +142,7 @@ extern void reserve_top_address(unsigned long reserve);
extern int fixmaps_set;

extern pte_t *kmap_pte;
-extern pgprot_t kmap_prot;
+#define kmap_prot PAGE_KERNEL
extern pte_t *pkmap_page_table;

void __native_set_fixmap(enum fixed_addresses idx, pte_t pte);
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index cb4ef3de61f9..a4bb1c7ab65e 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -388,7 +388,6 @@ repeat:
}

pte_t *kmap_pte;
-pgprot_t kmap_prot;

static inline pte_t *kmap_get_fixmap_pte(unsigned long vaddr)
{
@@ -405,8 +404,6 @@ static void __init kmap_init(void)
*/
kmap_vstart = __fix_to_virt(FIX_KMAP_BEGIN);
kmap_pte = kmap_get_fixmap_pte(kmap_vstart);
-
- kmap_prot = PAGE_KERNEL;
}

#ifdef CONFIG_HIGHMEM
--
2.5.0

2015-12-15 09:40:25

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Mon 2015-12-14 14:25:10, Andy Lutomirski wrote:
> On Mon, Dec 14, 2015 at 1:24 PM, Arjan van de Ven <[email protected]> wrote:
> >
> >> That's weird. The only API to do that seems to be manually setting
> >> kmap_prot to _PAGE_KERNEL_EXEC, and nothing does that. (Why is
> >> kmap_prot a variable on x86 at all? It has exactly one writer, and
> >> that's the code that initializes it in the first place. Shouldn't we
> >> #define kmap_prot _PAGE_KERNEL?
> >
> >
> > iirc it changes based on runtime detection of NX capability
> >
>
> Maybe it did, but if it still does, I can't find the code.
>
> What *does* change is __supported_pte_mask. If we're willing to make
> disable_nx work a little less well, we could try to initialize
> __supported_pte_mask from the very beginning. (We currently seem to
> detect and enable NX even before we enable paging.) I suspect that
> Pavel is seeing a kmap mapping left over from so early that it didn't
> have NX set (killed by massage_pgprot).

I tried applying:

[PATCH 1/2] x86_32/mm: Set NX in __supported_pte_mask before enabling
paging

but I still get

[ 2.685402] ------------[ cut here ]------------
[ 2.688649] WARNING: CPU: 0 PID: 1 at
arch/x86/mm/dump_pagetables.c:225 note_
page+0x5ec/0x790()
[ 2.691897] x86/mm: Found insecure W+X mapping at address
ffe69000/0xffe69000
[ 2.695090] Modules linked in:

Best regards,
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2015-12-15 13:30:31

by Arjan van de Ven

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On 12/14/2015 11:56 PM, Pavel Machek wrote:
> On Mon 2015-12-14 13:24:08, Arjan van de Ven wrote:
>>
>>> That's weird. The only API to do that seems to be manually setting
>>> kmap_prot to _PAGE_KERNEL_EXEC, and nothing does that. (Why is
>>> kmap_prot a variable on x86 at all? It has exactly one writer, and
>>> that's the code that initializes it in the first place. Shouldn't we
>>> #define kmap_prot _PAGE_KERNEL?
>>
>> iirc it changes based on runtime detection of NX capability
>
> Huh. Is it possible that core duo is so old that it has no NX?

really stupid question I guess, but is PAE on ?
(64 bit pagetables are required for NX)

2015-12-15 14:08:41

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Tue 2015-12-15 05:26:00, Arjan van de Ven wrote:
> On 12/14/2015 11:56 PM, Pavel Machek wrote:
> >On Mon 2015-12-14 13:24:08, Arjan van de Ven wrote:
> >>
> >>>That's weird. The only API to do that seems to be manually setting
> >>>kmap_prot to _PAGE_KERNEL_EXEC, and nothing does that. (Why is
> >>>kmap_prot a variable on x86 at all? It has exactly one writer, and
> >>>that's the code that initializes it in the first place. Shouldn't we
> >>>#define kmap_prot _PAGE_KERNEL?
> >>
> >>iirc it changes based on runtime detection of NX capability
> >
> >Huh. Is it possible that core duo is so old that it has no NX?
>
> really stupid question I guess, but is PAE on ?
> (64 bit pagetables are required for NX)
>

CONFIG_X86_PAE=y

pavel@duo:/data/l/linux$ cat /proc/cpuinfo | grep pae
flags : fpu vme de pse tsc msr pae mce cx8 apic
sep mtrr pge mca cmov clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe
nx constant_tsc arch_perfmon bts aperfmperf pni monitor vmx est tm2
xtpr pdcm dtherm

But it still says:

address sizes : 32 bits physical, 32 bits virtual

I thought pae would be 36bit virtual?

I'm attaching /proc/config.gz..
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Attachments:

(No filename) (1.27 kB)
config.gz (23.83 kB)
Download all attachments

2015-12-15 16:29:29

by H. Peter Anvin

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On 12/15/15 06:08, Pavel Machek wrote:
>
> But it still says:
>
> address sizes : 32 bits physical, 32 bits virtual
>
> I thought pae would be 36bit virtual?
>

It should be unless the CPU reports otherwise, which your CPU probably
does (a CPUID dump might be useful.)

-hpa

2015-12-15 17:45:17

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Tue 2015-12-15 08:28:12, H. Peter Anvin wrote:
> On 12/15/15 06:08, Pavel Machek wrote:
> >
> > But it still says:
> >
> > address sizes : 32 bits physical, 32 bits virtual
> >
> > I thought pae would be 36bit virtual?
> >
>
> It should be unless the CPU reports otherwise, which your CPU probably
> does (a CPUID dump might be useful.)

Ok, here you go :-).
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Attachments:

(No filename) (519.00 B)
delme.cpuinfo (26.34 kB)
cpuinfo (1.41 kB)
Download all attachments

2015-12-15 17:45:43

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Tue, Dec 15, 2015 at 1:40 AM, Pavel Machek <[email protected]> wrote:
>
> I tried applying:
>
> [PATCH 1/2] x86_32/mm: Set NX in __supported_pte_mask before enabling
> paging
>
> but I still get
>
> [ 2.691897] x86/mm: Found insecure W+X mapping at address ffe69000/0xffe69000

This may be an insane suggestion, but how about we try to detect when
that entry gets set, rather than after the fact.

Something really brute-force like

diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 6ec0c8b2e9df..538c9bb239b9 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -337,6 +337,13 @@ static inline pmd_t pmd_clear_soft_dirty(pmd_t pmd)

#endif /* CONFIG_HAVE_ARCH_SOFT_DIRTY */

+static inline int kernel_write_execute_prot(pgprotval_t protval)
+{
+ return !(protval & _PAGE_USER) &&
+ !(protval & _PAGE_NX) &&
+ (protval & _PAGE_RW);
+}
+
/*
* Mask out unsupported bits in a present pgprot. Non-present pgprots
* can use those bits for other purposes, so leave them be.
@@ -345,8 +352,10 @@ static inline pgprotval_t massage_pgprot(pgprot_t pgprot)
{
pgprotval_t protval = pgprot_val(pgprot);

- if (protval & _PAGE_PRESENT)
+ if (protval & _PAGE_PRESENT) {
protval &= __supported_pte_mask;
+ WARN_ON_ONCE(kernel_write_execute_prot(protval));
+ }

return protval;
}

or similar?

The above is entirely untested. Maybe it doesn't compile. Or boot. Or work.

Linus

2015-12-15 18:30:14

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Tue, Dec 15, 2015 at 09:45:40AM -0800, Linus Torvalds wrote:
> On Tue, Dec 15, 2015 at 1:40 AM, Pavel Machek <[email protected]> wrote:
> >
> > I tried applying:
> >
> > [PATCH 1/2] x86_32/mm: Set NX in __supported_pte_mask before enabling
> > paging
> >
> > but I still get
> >
> > [ 2.691897] x86/mm: Found insecure W+X mapping at address ffe69000/0xffe69000
>
> This may be an insane suggestion, but how about we try to detect when
> that entry gets set, rather than after the fact.
>
> Something really brute-force like
>
> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
> index 6ec0c8b2e9df..538c9bb239b9 100644
> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -337,6 +337,13 @@ static inline pmd_t pmd_clear_soft_dirty(pmd_t pmd)
>
> #endif /* CONFIG_HAVE_ARCH_SOFT_DIRTY */
>
> +static inline int kernel_write_execute_prot(pgprotval_t protval)
> +{
> + return !(protval & _PAGE_USER) &&
> + !(protval & _PAGE_NX) &&

Shouldn't this be without a "!"? AFAIU, we want _PAGE_NX | _PAGE_RW?

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.

2015-12-15 18:41:14

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Tue, Dec 15, 2015 at 9:45 AM, Linus Torvalds
<[email protected]> wrote:
> On Tue, Dec 15, 2015 at 1:40 AM, Pavel Machek <[email protected]> wrote:
>>
>> I tried applying:
>>
>> [PATCH 1/2] x86_32/mm: Set NX in __supported_pte_mask before enabling
>> paging
>>
>> but I still get
>>
>> [ 2.691897] x86/mm: Found insecure W+X mapping at address ffe69000/0xffe69000
>
> This may be an insane suggestion, but how about we try to detect when
> that entry gets set, rather than after the fact.
>
> Something really brute-force like
>
> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
> index 6ec0c8b2e9df..538c9bb239b9 100644
> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -337,6 +337,13 @@ static inline pmd_t pmd_clear_soft_dirty(pmd_t pmd)
>
> #endif /* CONFIG_HAVE_ARCH_SOFT_DIRTY */
>
> +static inline int kernel_write_execute_prot(pgprotval_t protval)
> +{
> + return !(protval & _PAGE_USER) &&
> + !(protval & _PAGE_NX) &&
> + (protval & _PAGE_RW);
> +}
> +
> /*
> * Mask out unsupported bits in a present pgprot. Non-present pgprots
> * can use those bits for other purposes, so leave them be.
> @@ -345,8 +352,10 @@ static inline pgprotval_t massage_pgprot(pgprot_t pgprot)
> {
> pgprotval_t protval = pgprot_val(pgprot);
>
> - if (protval & _PAGE_PRESENT)
> + if (protval & _PAGE_PRESENT) {
> protval &= __supported_pte_mask;
> + WARN_ON_ONCE(kernel_write_execute_prot(protval));

Shouldn't we switch those two lines? Arguably trying to set rwx
permissions on !PAE is a bug even if it makes no difference, whereas
setting rw- and getting rwx because we don't have NX support on the
platform or kernel build doesn't indicate a bug.

Anyway, I still think that we should apply my patches for 4.5 because
I think they're cleanups, but apparently I guessed wrong as to what
was causing Pavel's issue. But your patch would help diagnose it.

--Andy

2015-12-15 19:06:40

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Tue, Dec 15, 2015 at 10:30 AM, Borislav Petkov <[email protected]> wrote:
> On Tue, Dec 15, 2015 at 09:45:40AM -0800, Linus Torvalds wrote:
>>
>> +static inline int kernel_write_execute_prot(pgprotval_t protval)
>> +{
>> + return !(protval & _PAGE_USER) &&
>> + !(protval & _PAGE_NX) &&
>
> Shouldn't this be without a "!"? AFAIU, we want _PAGE_NX | _PAGE_RW?

We want to *warn* about PAGE_RW without PAGE_NX. No?

Linus

2015-12-15 19:08:25

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Tue, Dec 15, 2015 at 10:40 AM, Andy Lutomirski <[email protected]> wrote:
>
> Shouldn't we switch those two lines?

No. This is for debugging Pavel's issue. If somehow the
__supported_pte_mask ends up having NX clear again (memory scribble,
whatever), we very explicitly would want to catch that.

I'm *not* suggesting this as a generic patch to be applied anywhere
else. That patch is meant to debug Pavel's particular setup, and
nothing else.

Linus

2015-12-15 19:15:11

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Tue, Dec 15, 2015 at 11:06:38AM -0800, Linus Torvalds wrote:
> We want to *warn* about PAGE_RW without PAGE_NX. No?

Ah yes, we do:

note_page:

...

if (st->check_wx && (pr & _PAGE_RW) && !(pr & _PAGE_NX)) {

I got confused, sorry.

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.

2015-12-15 20:58:41

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

Hi!

> > I tried applying:
> >
> > [PATCH 1/2] x86_32/mm: Set NX in __supported_pte_mask before enabling
> > paging
> >
> > but I still get
> >
> > [ 2.691897] x86/mm: Found insecure W+X mapping at address ffe69000/0xffe69000
>
> This may be an insane suggestion, but how about we try to detect when
> that entry gets set, rather than after the fact.
>
> Something really brute-force like
>
> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
> index 6ec0c8b2e9df..538c9bb239b9 100644
> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -337,6 +337,13 @@ static inline pmd_t pmd_clear_soft_dirty(pmd_t pmd)
>
> #endif /* CONFIG_HAVE_ARCH_SOFT_DIRTY */
>
> +static inline int kernel_write_execute_prot(pgprotval_t protval)
> +{
> + return !(protval & _PAGE_USER) &&
> + !(protval & _PAGE_NX) &&
> + (protval & _PAGE_RW);
> +}
...
> + if (protval & _PAGE_PRESENT) {
> protval &= __supported_pte_mask;
> + WARN_ON_ONCE(kernel_write_execute_prot(protval));
> + }
>
> return protval;
> }
>
> or similar?
>
> The above is entirely untested. Maybe it doesn't compile. Or
> boot. Or work.

Well, with two extra spaces at each line, it does not apply :-).

I applied it by hand, and the output is:

[ 0.000000] MTRR variable ranges enabled:
[ 0.000000] 0 base 000000000 mask F80000000 write-back
[ 0.000000] 1 base 080000000 mask FC0000000 write-back
[ 0.000000] 2 base 0BF700000 mask FFFF00000 uncachable
[ 0.000000] 3 base 0BF800000 mask FFF800000 uncachable
[ 0.000000] 4 disabled
[ 0.000000] 5 disabled
[ 0.000000] 6 disabled
[ 0.000000] 7 disabled
[ 0.000000] x86/PAT: PAT not supported by CPU.
[ 0.000000] initial memory mapped: [mem 0x00000000-0x05bfffff]
[ 0.000000] Base memory trampoline at [c009b000] 9b000 size 16384
[ 0.000000] ------------[ cut here ]------------
[ 0.000000] WARNING: CPU: 0 PID: 0 at
./arch/x86/include/asm/pgtable.h:357 kernel_physical_mapping_init+0x
256/0x395()
[ 0.000000] Modules linked in:
[ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.4.0-rc5+ #137
[ 0.000000] Hardware name: LENOVO 17097HU/17097HU, BIOS 7BETD8WW
(2.19 ) 03/31/2011
[ 0.000000] 00000000 00000000 c4e63e90 c42baaf8 00000000 c4e63eac
c404066b 00000165
[ 0.000000] c4f134da 00000000 00000000 00000000 c4e63ebc c404070f
00000009 00000000
[ 0.000000] c4e63f18 c4f134da c4e63f00 00000000 00000000 00000000
00000000 00000000
[ 0.000000] Call Trace:
[ 0.000000] [<c42baaf8>] dump_stack+0x41/0x59
[ 0.000000] [<c404066b>] warn_slowpath_common+0x6b/0xa0
[ 0.000000] [<c4f134da>] ?
kernel_physical_mapping_init+0x256/0x395
[ 0.000000] [<c404070f>] warn_slowpath_null+0xf/0x20
[ 0.000000] [<c4f134da>] kernel_physical_mapping_init+0x256/0x395
[ 0.000000] [<c4a4de21>] init_memory_mapping+0x191/0x300
[ 0.000000] [<c4f12d96>] init_mem_mapping+0xe7/0x1f3
[ 0.000000] [<c4f12d96>] ? init_mem_mapping+0xe7/0x1f3
[ 0.000000] [<c4f065ef>] setup_arch+0x659/0x8ca
[ 0.000000] [<c4f0480e>] start_kernel+0xbb/0x360
[ 0.000000] [<c4f042d4>] i386_start_kernel+0x82/0x86
[ 0.000000] ---[ end trace e117245cd61feaf1 ]---
[ 0.000000] BRK [0x0566a000, 0x0566afff] PGTABLE
[ 0.000000] BRK [0x0566b000, 0x0566bfff] PGTABLE
[ 0.000000] BRK [0x0566c000, 0x0566cfff] PGTABLE

I'll take a look if I can figure out what it means...
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2015-12-15 21:12:36

[permalink] [raw]

Subject: 4.4.-rc5: lguest causes ugly warn on: 5 W+X pages found

Hi!

> > or similar?
> >
> > The above is entirely untested. Maybe it doesn't compile. Or
> > boot. Or work.
>
> Well, with two extra spaces at each line, it does not apply :-).
>
> I applied it by hand, and the output is:
>
> [ 0.000000] MTRR variable ranges enabled:
...> [ 0.000000] BRK [0x0566c000, 0x0566cfff] PGTABLE
>
> I'll take a look if I can figure out what it means...

Wait, there's more in the log.

[ 1.952146] Bluetooth: HCI UART protocol H4 registered
[ 1.954335] Bluetooth: HCI UART protocol BCSP registered
[ 1.956750] usbcore: registered new interface driver btusb
[ 1.958953] ------------[ cut here ]------------
[ 1.961149] WARNING: CPU: 1 PID: 1 at
./arch/x86/include/asm/pgtable.h:357
vmap_page_range_noflush+0x1f0/0x280()
[ 1.963511] Modules linked in:
[ 1.965849] CPU: 1 PID: 1 Comm: swapper/0 Tainted: G W
4.4.0-rc5+ #137
[ 1.968230] Hardware name: LENOVO 17097HU/17097HU, BIOS 7BETD8WW
(2.19 ) 03/31/2011
[ 1.970593] 00000001 00000000 f5cffe64 c42baaf8 00000000 f5cffe80
c404066b 00000165
[ 1.973103] c40fbe70 00000163 00000000 00000000 f5cffe90 c404070f
00000009 00000000
[ 1.975670] f5cffee0 c40fbe70 c4f88348 00000000 ffe6dfff ffe6e000
c4f8a018 ffe6dfff
[ 1.978304] Call Trace:
[ 1.980882] [<c42baaf8>] dump_stack+0x41/0x59
[ 1.983464] [<c404066b>] warn_slowpath_common+0x6b/0xa0
[ 1.986053] [<c40fbe70>] ? vmap_page_range_noflush+0x1f0/0x280
[ 1.988625] [<c404070f>] warn_slowpath_null+0xf/0x20
[ 1.991154] [<c40fbe70>] vmap_page_range_noflush+0x1f0/0x280
[ 1.993676] [<c40fbf2b>] map_vm_area+0x2b/0x40
[ 1.996153] [<c4f2c795>] init+0xf8/0x1a4
[ 1.998591] [<c4f2c69d>] ? edac_init+0x67/0x67
[ 2.001014] [<c4000442>] do_one_initcall+0xc2/0x1c0
[ 2.003391] [<c4f044e3>] ? initcall_blacklist+0x97/0x97
[ 2.005815] [<c4f044e3>] ? initcall_blacklist+0x97/0x97
[ 2.008161] [<c4051546>] ?
__usermodehelper_set_disable_depth+0x36/0x40
[ 2.010518] [<c407d4a6>] ? up_write+0x16/0x40
[ 2.012817] [<c4f04ba3>] kernel_init_freeable+0xf0/0x16d
[ 2.015078] [<c4f04ba3>] ? kernel_init_freeable+0xf0/0x16d
[ 2.017386] [<c4a4d9c8>] kernel_init+0x8/0xc0
[ 2.019661] [<c4a54149>] ret_from_kernel_thread+0x21/0x38
[ 2.021932] [<c4a4d9c0>] ? rest_init+0xa0/0xa0
[ 2.024168] ---[ end trace e117245cd61feaf2 ]---
[ 2.026383] lguest: mapped switcher at ffe69000
[ 2.028958] sdhci: Secure Digital Host Controller Interface driver

...which I don't understand; did not we say warn on _once_?
... Um. But I think we have a winner: "lguest: mapped switcher at
ffe69000".

Rusty, does the switcher need to be W+X?

And yes, I have lguest enabled, not sure why.

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2015-12-15 21:34:09

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Tue, Dec 15, 2015 at 09:58:35PM +0100, Pavel Machek wrote:
> [ 0.000000] Base memory trampoline at [c009b000] 9b000 size 16384
> [ 0.000000] ------------[ cut here ]------------
> [ 0.000000] WARNING: CPU: 0 PID: 0 at
> ./arch/x86/include/asm/pgtable.h:357 kernel_physical_mapping_init+0x
> 256/0x395()
> [ 0.000000] Modules linked in:
> [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.4.0-rc5+ #137
> [ 0.000000] Hardware name: LENOVO 17097HU/17097HU, BIOS 7BETD8WW
> (2.19 ) 03/31/2011
> [ 0.000000] 00000000 00000000 c4e63e90 c42baaf8 00000000 c4e63eac
> c404066b 00000165
> [ 0.000000] c4f134da 00000000 00000000 00000000 c4e63ebc c404070f
> 00000009 00000000
> [ 0.000000] c4e63f18 c4f134da c4e63f00 00000000 00000000 00000000
> 00000000 00000000
> [ 0.000000] Call Trace:
> [ 0.000000] [<c42baaf8>] dump_stack+0x41/0x59
> [ 0.000000] [<c404066b>] warn_slowpath_common+0x6b/0xa0
> [ 0.000000] [<c4f134da>] ?
> kernel_physical_mapping_init+0x256/0x395
> [ 0.000000] [<c404070f>] warn_slowpath_null+0xf/0x20
> [ 0.000000] [<c4f134da>] kernel_physical_mapping_init+0x256/0x395
> [ 0.000000] [<c4a4de21>] init_memory_mapping+0x191/0x300
> [ 0.000000] [<c4f12d96>] init_mem_mapping+0xe7/0x1f3

Looks like the ISA range to me:

init_mem_mapping:

...

/* the ISA range is always mapped regardless of memory holes */
init_memory_mapping(0, ISA_END_ADDRESS);

Does that kernel_physical_mapping_init() even pay attention to
__supported_pte_mask and thus _PAGE_NX? I don't see it.

Hmm, not really:

pgprot_t init_prot = __pgprot(PTE_IDENT_ATTR | _PAGE_PSE);

...

prot = PAGE_KERNEL_LARGE_EXEC;

...

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.

2015-12-15 22:07:14

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Tue 2015-12-15 22:33:59, Borislav Petkov wrote:
> On Tue, Dec 15, 2015 at 09:58:35PM +0100, Pavel Machek wrote:
> > [ 0.000000] Base memory trampoline at [c009b000] 9b000 size 16384
> > [ 0.000000] ------------[ cut here ]------------
> > [ 0.000000] WARNING: CPU: 0 PID: 0 at
> > ./arch/x86/include/asm/pgtable.h:357 kernel_physical_mapping_init+0x
> > 256/0x395()
> > [ 0.000000] Modules linked in:
> > [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.4.0-rc5+ #137
> > [ 0.000000] Hardware name: LENOVO 17097HU/17097HU, BIOS 7BETD8WW
> > (2.19 ) 03/31/2011
> > [ 0.000000] 00000000 00000000 c4e63e90 c42baaf8 00000000 c4e63eac
> > c404066b 00000165
> > [ 0.000000] c4f134da 00000000 00000000 00000000 c4e63ebc c404070f
> > 00000009 00000000
> > [ 0.000000] c4e63f18 c4f134da c4e63f00 00000000 00000000 00000000
> > 00000000 00000000
> > [ 0.000000] Call Trace:
> > [ 0.000000] [<c42baaf8>] dump_stack+0x41/0x59
> > [ 0.000000] [<c404066b>] warn_slowpath_common+0x6b/0xa0
> > [ 0.000000] [<c4f134da>] ?
> > kernel_physical_mapping_init+0x256/0x395
> > [ 0.000000] [<c404070f>] warn_slowpath_null+0xf/0x20
> > [ 0.000000] [<c4f134da>] kernel_physical_mapping_init+0x256/0x395
> > [ 0.000000] [<c4a4de21>] init_memory_mapping+0x191/0x300
> > [ 0.000000] [<c4f12d96>] init_mem_mapping+0xe7/0x1f3
>
> Looks like the ISA range to me:
>
> init_mem_mapping:
>
> ...
>
> /* the ISA range is always mapped regardless of memory holes */
> init_memory_mapping(0, ISA_END_ADDRESS);
>
> Does that kernel_physical_mapping_init() even pay attention to
> __supported_pte_mask and thus _PAGE_NX? I don't see it.

Mystery solved, and prize goes to the lguest.

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2015-12-15 22:15:38

[permalink] [raw]

Subject: Re: 4.4-rc5: ugly warn on: 5 W+X pages found

On Tue, Dec 15, 2015 at 11:07:09PM +0100, Pavel Machek wrote:
> Mystery solved, and prize goes to the lguest.

But that splat has nowhere lguest in it. Are you saying, you don't see
any more splats if you disable lguest?

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.

2015-12-16 02:25:27

by Rusty Russell

[permalink] [raw]

Subject: Re: 4.4.-rc5: lguest causes ugly warn on: 5 W+X pages found

Pavel Machek <[email protected]> writes:
> Hi!
>
>> > or similar?
>> >
>> > The above is entirely untested. Maybe it doesn't compile. Or
>> > boot. Or work.
>>
>> Well, with two extra spaces at each line, it does not apply :-).
>>
>> I applied it by hand, and the output is:
>>
>> [ 0.000000] MTRR variable ranges enabled:
> ...> [ 0.000000] BRK [0x0566c000, 0x0566cfff] PGTABLE
>>
>> I'll take a look if I can figure out what it means...
>
> Wait, there's more in the log.
>
> [ 1.952146] Bluetooth: HCI UART protocol H4 registered
> [ 1.954335] Bluetooth: HCI UART protocol BCSP registered
> [ 1.956750] usbcore: registered new interface driver btusb
> [ 1.958953] ------------[ cut here ]------------
> [ 1.961149] WARNING: CPU: 1 PID: 1 at
> ./arch/x86/include/asm/pgtable.h:357
> vmap_page_range_noflush+0x1f0/0x280()
> [ 1.963511] Modules linked in:
> [ 1.965849] CPU: 1 PID: 1 Comm: swapper/0 Tainted: G W
> 4.4.0-rc5+ #137
> [ 1.968230] Hardware name: LENOVO 17097HU/17097HU, BIOS 7BETD8WW
> (2.19 ) 03/31/2011
> [ 1.970593] 00000001 00000000 f5cffe64 c42baaf8 00000000 f5cffe80
> c404066b 00000165
> [ 1.973103] c40fbe70 00000163 00000000 00000000 f5cffe90 c404070f
> 00000009 00000000
> [ 1.975670] f5cffee0 c40fbe70 c4f88348 00000000 ffe6dfff ffe6e000
> c4f8a018 ffe6dfff
> [ 1.978304] Call Trace:
> [ 1.980882] [<c42baaf8>] dump_stack+0x41/0x59
> [ 1.983464] [<c404066b>] warn_slowpath_common+0x6b/0xa0
> [ 1.986053] [<c40fbe70>] ? vmap_page_range_noflush+0x1f0/0x280
> [ 1.988625] [<c404070f>] warn_slowpath_null+0xf/0x20
> [ 1.991154] [<c40fbe70>] vmap_page_range_noflush+0x1f0/0x280
> [ 1.993676] [<c40fbf2b>] map_vm_area+0x2b/0x40
> [ 1.996153] [<c4f2c795>] init+0xf8/0x1a4
> [ 1.998591] [<c4f2c69d>] ? edac_init+0x67/0x67
> [ 2.001014] [<c4000442>] do_one_initcall+0xc2/0x1c0
> [ 2.003391] [<c4f044e3>] ? initcall_blacklist+0x97/0x97
> [ 2.005815] [<c4f044e3>] ? initcall_blacklist+0x97/0x97
> [ 2.008161] [<c4051546>] ?
> __usermodehelper_set_disable_depth+0x36/0x40
> [ 2.010518] [<c407d4a6>] ? up_write+0x16/0x40
> [ 2.012817] [<c4f04ba3>] kernel_init_freeable+0xf0/0x16d
> [ 2.015078] [<c4f04ba3>] ? kernel_init_freeable+0xf0/0x16d
> [ 2.017386] [<c4a4d9c8>] kernel_init+0x8/0xc0
> [ 2.019661] [<c4a54149>] ret_from_kernel_thread+0x21/0x38
> [ 2.021932] [<c4a4d9c0>] ? rest_init+0xa0/0xa0
> [ 2.024168] ---[ end trace e117245cd61feaf2 ]---
> [ 2.026383] lguest: mapped switcher at ffe69000
> [ 2.028958] sdhci: Secure Digital Host Controller Interface driver
>
> ...which I don't understand; did not we say warn on _once_?
> ... Um. But I think we have a winner: "lguest: mapped switcher at
> ffe69000".
>
> Rusty, does the switcher need to be W+X?
>
> And yes, I have lguest enabled, not sure why.

No. The layout is "<text page> <per-cpu-stack-pages>..." and I lazily
did that as a single
map_vm_area(switcher_vma, PAGE_KERNEL_EXEC, lg_switcher_pages);

This boots, does it solve the problem?

Thanks!
Rusty.

From: Rusty Russell <[email protected]>
Subject: lguest: map switcher text R/O.

Pavel noted that lguest maps the switcher code executable and
read-write. This is a bad idea for any kernel text, but particularly
for text mapped at a fixed address.

Create two vmas, one for the text (PAGE_KERNEL_RX) and another for the
stacks (PAGE_KERNEL). Use VM_NO_GUARD to map them adjacent (as
expected by the rest of the code).

Reported-by: Pavel Machek <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>

diff --git a/arch/x86/include/asm/lguest.h b/arch/x86/include/asm/lguest.h
index 3bbc07a57a31..73d0c9b92087 100644
--- a/arch/x86/include/asm/lguest.h
+++ b/arch/x86/include/asm/lguest.h
@@ -12,7 +12,9 @@
#define GUEST_PL 1

/* Page for Switcher text itself, then two pages per cpu */
-#define TOTAL_SWITCHER_PAGES (1 + 2 * nr_cpu_ids)
+#define SWITCHER_TEXT_PAGES (1)
+#define SWITCHER_STACK_PAGES (2 * nr_cpu_ids)
+#define TOTAL_SWITCHER_PAGES (SWITCHER_TEXT_PAGES + SWITCHER_STACK_PAGES)

/* Where we map the Switcher, in both Host and Guest. */
extern unsigned long switcher_addr;
diff --git a/drivers/lguest/core.c b/drivers/lguest/core.c
index 312ffd3d0017..021915baef35 100644
--- a/drivers/lguest/core.c
+++ b/drivers/lguest/core.c
@@ -22,7 +22,8 @@

unsigned long switcher_addr;
struct page **lg_switcher_pages;
-static struct vm_struct *switcher_vma;
+static struct vm_struct *switcher_text_vma;
+static struct vm_struct *switcher_stacks_vma;

/* This One Big lock protects all inter-guest data structures. */
DEFINE_MUTEX(lguest_lock);
@@ -83,54 +84,80 @@ static __init int map_switcher(void)
}

/*
+ * Copy in the compiled-in Switcher code (from x86/switcher_32.S).
+ * It goes in the first page, which we map in momentarily.
+ */
+ memcpy(kmap(lg_switcher_pages[0]), start_switcher_text,
+ end_switcher_text - start_switcher_text);
+ kunmap(lg_switcher_pages[0]);
+
+ /*
* We place the Switcher underneath the fixmap area, which is the
* highest virtual address we can get. This is important, since we
* tell the Guest it can't access this memory, so we want its ceiling
* as high as possible.
*/
- switcher_addr = FIXADDR_START - (TOTAL_SWITCHER_PAGES+1)*PAGE_SIZE;
+ switcher_addr = FIXADDR_START - TOTAL_SWITCHER_PAGES*PAGE_SIZE;

/*
- * Now we reserve the "virtual memory area" we want. We might
- * not get it in theory, but in practice it's worked so far.
- * The end address needs +1 because __get_vm_area allocates an
- * extra guard page, so we need space for that.
+ * Now we reserve the "virtual memory area"s we want. We might
+ * not get them in theory, but in practice it's worked so far.
+ *
+ * We want the switcher text to be read-only and executable, and
+ * the stacks to be read-write and non-executable.
*/
- switcher_vma = __get_vm_area(TOTAL_SWITCHER_PAGES * PAGE_SIZE,
- VM_ALLOC, switcher_addr, switcher_addr
- + (TOTAL_SWITCHER_PAGES+1) * PAGE_SIZE);
- if (!switcher_vma) {
+ switcher_text_vma = __get_vm_area(PAGE_SIZE, VM_ALLOC|VM_NO_GUARD,
+ switcher_addr,
+ switcher_addr + PAGE_SIZE);
+
+ if (!switcher_text_vma) {
err = -ENOMEM;
printk("lguest: could not map switcher pages high\n");
goto free_pages;
}

+ switcher_stacks_vma = __get_vm_area(SWITCHER_STACK_PAGES * PAGE_SIZE,
+ VM_ALLOC|VM_NO_GUARD,
+ switcher_addr + PAGE_SIZE,
+ switcher_addr + TOTAL_SWITCHER_PAGES * PAGE_SIZE);
+ if (!switcher_stacks_vma) {
+ err = -ENOMEM;
+ printk("lguest: could not map switcher pages high\n");
+ goto free_text_vma;
+ }
+
/*
* This code actually sets up the pages we've allocated to appear at
* switcher_addr. map_vm_area() takes the vma we allocated above, the
- * kind of pages we're mapping (kernel pages), and a pointer to our
- * array of struct pages.
+ * kind of pages we're mapping (kernel text pages and kernel writable
+ * pages respectively), and a pointer to our array of struct pages.
*/
- err = map_vm_area(switcher_vma, PAGE_KERNEL_EXEC, lg_switcher_pages);
+ err = map_vm_area(switcher_text_vma, PAGE_KERNEL_RX, lg_switcher_pages);
+ if (err) {
+ printk("lguest: text map_vm_area failed: %i\n", err);
+ goto free_vmas;
+ }
+
+ err = map_vm_area(switcher_stacks_vma, PAGE_KERNEL,
+ lg_switcher_pages + SWITCHER_TEXT_PAGES);
if (err) {
- printk("lguest: map_vm_area failed: %i\n", err);
- goto free_vma;
+ printk("lguest: stacks map_vm_area failed: %i\n", err);
+ goto free_vmas;
}

/*
* Now the Switcher is mapped at the right address, we can't fail!
- * Copy in the compiled-in Switcher code (from x86/switcher_32.S).
*/
- memcpy(switcher_vma->addr, start_switcher_text,
- end_switcher_text - start_switcher_text);
-
printk(KERN_INFO "lguest: mapped switcher at %p\n",
- switcher_vma->addr);
+ switcher_text_vma->addr);
/* And we succeeded... */
return 0;

-free_vma:
- vunmap(switcher_vma->addr);
+free_vmas:
+ /* Undoes map_vm_area and __get_vm_area */
+ vunmap(switcher_stacks_vma->addr);
+free_text_vma:
+ vunmap(switcher_text_vma->addr);
free_pages:
i = TOTAL_SWITCHER_PAGES;
free_some_pages:
@@ -148,7 +175,8 @@ static void unmap_switcher(void)
unsigned int i;

/* vunmap() undoes *both* map_vm_area() and __get_vm_area(). */
- vunmap(switcher_vma->addr);
+ vunmap(switcher_text_vma->addr);
+ vunmap(switcher_stacks_vma->addr);
/* Now we just need to free the pages we copied the switcher into */
for (i = 0; i < TOTAL_SWITCHER_PAGES; i++)
__free_pages(lg_switcher_pages[i], 0);

2015-12-16 08:10:52