Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Borislav Petkov]

On Fri, Sep 28, 2018 at 04:55:19PM +0200, Thomas Gleixner wrote:
> Sorry for the delay and thanks for the data. A quick diff did not reveal
> anything obvious. I'll have a closer look and we probably need more (other)
> information to nail that down.

Just a brain dump of what I've found out so far.

Commenting out the init_mem_mapping() call below:

void __init init_mem_mapping(void)
{
        unsigned long end;

	...

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

changes the address the warning reports to:

[    4.392870] x86/mm: Found insecure W+X mapping at address 0xc0000000/0xc0000000

but the machine boots fine otherwise.

Which begs the question: why do we direct-map the ISA range at
PAGE_OFFSET at all? Do we have to have virtual mappings of it at all? I
thought ISA devices don't need that but this is long before my time...

Then, the warning say too:

[    4.399804] x86/mm: Checked W+X mappings: FAILED, 252 W+X pages found.

and there really are 252 pages  (I counted) which are W+X:

---[ Kernel Mapping ]---
0xc0000000-0xc0001000           4K     RW                     x  pte
0xc0001000-0xc0099000         608K     RW                     x  pte
0xc0099000-0xc009a000           4K     ro                     NX pte
0xc009a000-0xc009b000           4K     ro                     x  pte
0xc009b000-0xc009d000           8K     RW                     NX pte
0xc009d000-0xc00a0000          12K     RW                     x  pte
0xc00a0000-0xc00a2000           8K     RW                     x  pte
0xc00a2000-0xc00b8000          88K     RW                     x  pte
0xc00b8000-0xc00c0000          32K     RW                     x  pte
0xc00c0000-0xc00f3000         204K     RW                     x  pte
0xc00f3000-0xc00fc000          36K     RW                     x  pte
0xc00fc000-0xc00fd000           4K     RW                     x  pte
0xc00fd000-0xc0100000          12K     RW                     x  pte
...

but I can't find where those guys appear from. Will be adding more debug
printks to track it down.

Anyway, just a dump of the current state...

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Paul Menzel]

Dear Borislav,

Am 03.10.2018 um 23:22 schrieb Borislav Petkov:
> On Fri, Sep 28, 2018 at 04:55:19PM +0200, Thomas Gleixner wrote:
>> Sorry for the delay and thanks for the data. A quick diff did not reveal
>> anything obvious. I'll have a closer look and we probably need more (other)
>> information to nail that down.
> 
> Just a brain dump of what I've found out so far.

Thank you for looking into this. On what board are you able to reproduce 
this? Do you build for 32-bit or 64-bit?


Kind regards,

Paul

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Borislav Petkov]

On Thu, Oct 04, 2018 at 05:11:17AM +0200, Paul Menzel wrote:
> Thank you for looking into this. On what board are you able to reproduce
> this? Do you build for 32-bit or 64-bit?

32-bit partition on an AMD F14h laptop.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Joerg Roedel]

On Wed, Oct 03, 2018 at 11:22:55PM +0200, Borislav Petkov wrote:
> On Fri, Sep 28, 2018 at 04:55:19PM +0200, Thomas Gleixner wrote:
> > Sorry for the delay and thanks for the data. A quick diff did not reveal
> > anything obvious. I'll have a closer look and we probably need more (other)
> > information to nail that down.

I also triggered this when working in the PTI-x32 code. It always
happens on a 32-bit PAE kernel for me.

Tracking it down I ended up in (iirc) arch/x86/mm/pageattr.c
	function static_protections():

		/*
		 * The BIOS area between 640k and 1Mb needs to be executable for
		 * PCI BIOS based config access (CONFIG_PCI_GOBIOS) support.
		 */
	#ifdef CONFIG_PCI_BIOS
		if (pcibios_enabled && within(pfn, BIOS_BEGIN >> PAGE_SHIFT, BIOS_END >> PAGE_SHIFT))
			pgprot_val(forbidden) |= _PAGE_NX;
	#endif

I think that is the reason we are seeing this in that configuration.


Regards,

	Joerg

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Borislav Petkov]

On Thu, Oct 04, 2018 at 10:03:21AM +0200, Joerg Roedel wrote:
> I also triggered this when working in the PTI-x32 code. It always
> happens on a 32-bit PAE kernel for me.
> 
> Tracking it down I ended up in (iirc) arch/x86/mm/pageattr.c
> 	function static_protections():
> 
> 		/*
> 		 * The BIOS area between 640k and 1Mb needs to be executable for
> 		 * PCI BIOS based config access (CONFIG_PCI_GOBIOS) support.
> 		 */
> 	#ifdef CONFIG_PCI_BIOS
> 		if (pcibios_enabled && within(pfn, BIOS_BEGIN >> PAGE_SHIFT, BIOS_END >> PAGE_SHIFT))
> 			pgprot_val(forbidden) |= _PAGE_NX;
> 	#endif
> 
> I think that is the reason we are seeing this in that configuration.

So looking at this, BIOS_BEGIN and BIOS_END is the same range as the ISA
range:

#define ISA_START_ADDRESS       0x000a0000
#define ISA_END_ADDRESS         0x00100000

#define BIOS_BEGIN              0x000a0000
#define BIOS_END                0x00100000


and I did try marking the ISA range RO in mark_rodata_ro() but the
machine wouldn't boot after. So I'm guessing BIOS needs to write there
some crap.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Paul Menzel]

[-- Attachment #1: Type: text/plain, Size: 1367 bytes --]

Dear Borislav,


On 10/04/18 10:14, Borislav Petkov wrote:
> On Thu, Oct 04, 2018 at 10:03:21AM +0200, Joerg Roedel wrote:
>> I also triggered this when working in the PTI-x32 code. It always
>> happens on a 32-bit PAE kernel for me.
>>
>> Tracking it down I ended up in (iirc) arch/x86/mm/pageattr.c
>> 	function static_protections():
>>
>> 		/*
>> 		 * The BIOS area between 640k and 1Mb needs to be executable for
>> 		 * PCI BIOS based config access (CONFIG_PCI_GOBIOS) support.
>> 		 */
>> 	#ifdef CONFIG_PCI_BIOS
>> 		if (pcibios_enabled && within(pfn, BIOS_BEGIN >> PAGE_SHIFT, BIOS_END >> PAGE_SHIFT))
>> 			pgprot_val(forbidden) |= _PAGE_NX;
>> 	#endif
>>
>> I think that is the reason we are seeing this in that configuration.
> 
> So looking at this, BIOS_BEGIN and BIOS_END is the same range as the ISA
> range:
> 
> #define ISA_START_ADDRESS       0x000a0000
> #define ISA_END_ADDRESS         0x00100000
> 
> #define BIOS_BEGIN              0x000a0000
> #define BIOS_END                0x00100000
> 
> and I did try marking the ISA range RO in mark_rodata_ro() but the 
> machine wouldn't boot after. So I'm guessing BIOS needs to write
> there some crap.

Do you have a commit, I could test. My board is supported by coreboot,
and maybe it boots, so I can check, if the warning is gone.


Kind regards,

Paul


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5174 bytes --]

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Joerg Roedel]

On Thu, Oct 04, 2018 at 10:14:38AM +0200, Borislav Petkov wrote:
> So looking at this, BIOS_BEGIN and BIOS_END is the same range as the ISA
> range:
> 
> #define ISA_START_ADDRESS       0x000a0000
> #define ISA_END_ADDRESS         0x00100000
> 
> #define BIOS_BEGIN              0x000a0000
> #define BIOS_END                0x00100000
> 
> 
> and I did try marking the ISA range RO in mark_rodata_ro() but the
> machine wouldn't boot after. So I'm guessing BIOS needs to write there
> some crap.

Yeah, that's what I also found out back then, the region needs to be WX.
So we can either leave with the warning, as we know it is harmless and
where it comes from or implement an exception in the checking code for
that region.

Regards,

	Joerg

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Borislav Petkov]

On Thu, Oct 04, 2018 at 10:43:18AM +0200, Joerg Roedel wrote:
> Yeah, that's what I also found out back then, the region needs to be WX.
> So we can either leave with the warning, as we know it is harmless and
> where it comes from or implement an exception in the checking code for
> that region.

The second thing is what I'm thinking too.

Or, a 3rd: not direct-map that first range at all. Commenting out the
ISA range mapping didn't have any adverse effects on my system here, for
example. But then those other mappings appeared:

https://lkml.kernel.org/r/20181003212255.GB28361@zn.tnic

for which I have no explanation yet how they came about.

This needs to be understood fully before we do anything. But it is
32-bit so it gets preempted by more important things all the time :)

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Borislav Petkov]

On Thu, Oct 04, 2018 at 10:40:49AM +0200, Paul Menzel wrote:
> Do you have a commit, I could test.

Not yet but I have a question for you: why are you running 32-bit and
haven't moved to 64-bit already?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Paul Menzel]

[-- Attachment #1: Type: text/plain, Size: 426 bytes --]

Dear Borislav,


On 10/04/18 10:49, Borislav Petkov wrote:
> On Thu, Oct 04, 2018 at 10:40:49AM +0200, Paul Menzel wrote:
>> Do you have a commit, I could test.
> 
> Not yet

I meant just the test you did.

> but I have a question for you: why are you running 32-bit and
> haven't moved to 64-bit already?

The SSD is also used in the Lenovo X60 and T60, which are
32-bit systems.


Kind regards,

Paul


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5174 bytes --]

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Borislav Petkov]

On Thu, Oct 04, 2018 at 10:59:18AM +0200, Paul Menzel wrote:
> I meant just the test you did.

https://lkml.kernel.org/r/20181003212255.GB28361@zn.tnic

> The SSD is also used in the Lenovo X60 and T60, which are
> 32-bit systems.

And what exactly is the problem when you access it on a 64-bit OS?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Paul Menzel]

[-- Attachment #1: Type: text/plain, Size: 793 bytes --]

Dear Borislav,


On 10/04/18 12:54, Borislav Petkov wrote:
> On Thu, Oct 04, 2018 at 10:59:18AM +0200, Paul Menzel wrote:
>> I meant just the test you did.
> 
> https://lkml.kernel.org/r/20181003212255.GB28361@zn.tnic

I see. But there you write, the machine does boot.

While here you write, it did not.

> and I did try marking the ISA range RO in mark_rodata_ro() but the
> machine wouldn't boot after.

>> The SSD is also used in the Lenovo X60 and T60, which are
>> 32-bit systems.
> 
> And what exactly is the problem when you access it on a 64-bit OS?

Sorry I do not understand the question. I carry the SSD drive with
me, and connect it to the ASRock E350M1 (64-bit) or to the Lenovo
X60 laptop and boot from it from both systems.


Kind regards,

Paul


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5174 bytes --]

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Borislav Petkov]

On Thu, Oct 04, 2018 at 01:00:42PM +0200, Paul Menzel wrote:
> While here you write, it did not.

Read again what I said:

> and I did try marking the ISA range RO in mark_rodata_ro() but the
> machine wouldn't boot after.

and the code I pasted has this:

	//      init_memory_mapping(0, ISA_END_ADDRESS);

which is disabling the direct mapping of the ISA range.

Two very different things.

And you don't absolutely need to try it because it would simply move the
warning to another address, just like it happened on my system. Because
looking at your dmesg, that E350M1 machine is very similar to the laptop
I have. But feel free if you have time on your hands... :)

> Sorry I do not understand the question. I carry the SSD drive with
> me, and connect it to the ASRock E350M1 (64-bit) or to the Lenovo
> X60 laptop and boot from it from both systems.

So it is an OS installation which you swap between two machines. I
admit, it is the first time I hear of such a use case. In that case,
yes, bitness does matter.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Thomas Gleixner]

On Thu, 4 Oct 2018, Joerg Roedel wrote:

> On Wed, Oct 03, 2018 at 11:22:55PM +0200, Borislav Petkov wrote:
> > On Fri, Sep 28, 2018 at 04:55:19PM +0200, Thomas Gleixner wrote:
> > > Sorry for the delay and thanks for the data. A quick diff did not reveal
> > > anything obvious. I'll have a closer look and we probably need more (other)
> > > information to nail that down.
> 
> I also triggered this when working in the PTI-x32 code. It always
> happens on a 32-bit PAE kernel for me.
> 
> Tracking it down I ended up in (iirc) arch/x86/mm/pageattr.c
> 	function static_protections():
> 
> 		/*
> 		 * The BIOS area between 640k and 1Mb needs to be executable for
> 		 * PCI BIOS based config access (CONFIG_PCI_GOBIOS) support.
> 		 */
> 	#ifdef CONFIG_PCI_BIOS
> 		if (pcibios_enabled && within(pfn, BIOS_BEGIN >> PAGE_SHIFT, BIOS_END >> PAGE_SHIFT))
> 			pgprot_val(forbidden) |= _PAGE_NX;
> 	#endif
> 
> I think that is the reason we are seeing this in that configuration.

Uurgh. Yes.

If pcibios is enabled and used, need to look at the gory details of that
first, then the W+X check has to exclude that region. We can't do much
about that.

Thanks,

	tglx

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Paul Menzel]

[-- Attachment #1: Type: text/plain, Size: 2595 bytes --]

Dear Thomas,


On 10/05/18 11:27, Thomas Gleixner wrote:
> On Thu, 4 Oct 2018, Joerg Roedel wrote:
> 
>> On Wed, Oct 03, 2018 at 11:22:55PM +0200, Borislav Petkov wrote:
>>> On Fri, Sep 28, 2018 at 04:55:19PM +0200, Thomas Gleixner wrote:
>>>> Sorry for the delay and thanks for the data. A quick diff did not reveal
>>>> anything obvious. I'll have a closer look and we probably need more (other)
>>>> information to nail that down.
>>
>> I also triggered this when working in the PTI-x32 code. It always
>> happens on a 32-bit PAE kernel for me.
>>
>> Tracking it down I ended up in (iirc) arch/x86/mm/pageattr.c
>> 	function static_protections():
>>
>> 		/*
>> 		 * The BIOS area between 640k and 1Mb needs to be executable for
>> 		 * PCI BIOS based config access (CONFIG_PCI_GOBIOS) support.
>> 		 */
>> 	#ifdef CONFIG_PCI_BIOS
>> 		if (pcibios_enabled && within(pfn, BIOS_BEGIN >> PAGE_SHIFT, BIOS_END >> PAGE_SHIFT))
>> 			pgprot_val(forbidden) |= _PAGE_NX;
>> 	#endif
>>
>> I think that is the reason we are seeing this in that configuration.
> 
> Uurgh. Yes.
> 
> If pcibios is enabled and used, need to look at the gory details of that
> first, then the W+X check has to exclude that region. We can't do much
> about that.

That would also explain, why it only happens with the SeaBIOS payload,
which sets up legacy BIOS calls. Using GRUB directly as payload, no BIOS
calls are set up.

Reading the Kconfig description of the PCI access mode, the BIOS should
only be used last.

> choice  
>         prompt "PCI access mode"
>         depends on X86_32 && PCI
>         default PCI_GOANY
>         ---help---
>           On PCI systems, the BIOS can be used to detect the PCI devices and
>           determine their configuration. However, some old PCI motherboards
>           have BIOS bugs and may crash if this is done. Also, some embedded
>           PCI-based systems don't have any BIOS at all. Linux can also try to
>           detect the PCI hardware directly without using the BIOS.
> 
>           With this option, you can specify how Linux should detect the
>           PCI devices. If you choose "BIOS", the BIOS will be used,
>           if you choose "Direct", the BIOS won't be used, and if you
>           choose "MMConfig", then PCI Express MMCONFIG will be used.
>           If you choose "Any", the kernel will try MMCONFIG, then the
>           direct access method and falls back to the BIOS if that doesn't
>           work. If unsure, go with the default, which is "Any".


Kind regards,

Paul


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5174 bytes --]

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Thomas Gleixner]

Paul,

On Fri, 5 Oct 2018, Paul Menzel wrote:
> On 10/05/18 11:27, Thomas Gleixner wrote:
> > If pcibios is enabled and used, need to look at the gory details of that
> > first, then the W+X check has to exclude that region. We can't do much
> > about that.
> 
> That would also explain, why it only happens with the SeaBIOS payload,
> which sets up legacy BIOS calls. Using GRUB directly as payload, no BIOS
> calls are set up.
> 
> Reading the Kconfig description of the PCI access mode, the BIOS should
> only be used last.

Correct. And looking at the dmesg you provided it is initialized:

[    0.441062] PCI: PCI BIOS area is rw and x. Use pci=nobios if you want it NX.
[    0.441062] PCI: PCI BIOS revision 2.10 entry at 0xffa40, last bus=3

Though I assume it's not really required, but this PCI BIOS thing is not
really well documented and there are some obsure usage sites involved.

Bjorn, do you have any insight or did you flush those memories long ago?

Anyway we need to exclude the BIOS area when the kernel sets the W+X on
purpose. Warning about that is bogus. I'll send out a patch soon.

Thanks,

	tglx

Re: x86/mm: Found insecure W+X mapping at address (ptrval)/0xc00a0000

[Bjorn Helgaas]

On Mon, Oct 8, 2018 at 2:37 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Paul,
>
> On Fri, 5 Oct 2018, Paul Menzel wrote:
> > On 10/05/18 11:27, Thomas Gleixner wrote:
> > > If pcibios is enabled and used, need to look at the gory details of that
> > > first, then the W+X check has to exclude that region. We can't do much
> > > about that.
> >
> > That would also explain, why it only happens with the SeaBIOS payload,
> > which sets up legacy BIOS calls. Using GRUB directly as payload, no BIOS
> > calls are set up.
> >
> > Reading the Kconfig description of the PCI access mode, the BIOS should
> > only be used last.
>
> Correct. And looking at the dmesg you provided it is initialized:
>
> [    0.441062] PCI: PCI BIOS area is rw and x. Use pci=nobios if you want it NX.
> [    0.441062] PCI: PCI BIOS revision 2.10 entry at 0xffa40, last bus=3
>
> Though I assume it's not really required, but this PCI BIOS thing is not
> really well documented and there are some obsure usage sites involved.
>
> Bjorn, do you have any insight or did you flush those memories long ago?

No, I don't.  I was never really involved with PCIBIOS.