2004-06-30 03:05:38

by Jamie Lokier

[permalink] [raw]
Subject: A question about PROT_NONE on Sparc and Sparc64

Hi folks,

I'm doing a survey of the different architectural implementations of
PROT_* flags for mmap() and mprotect(). I'm looking at linux-2.6.5.

The Sparc and Sparc64 implementations are very similar to plain x86:
read implies exec, exec implies read and write implies read.

(Aside: A comment in include/asm-sparc/pgtsrmmu.h says that finer-grained
access is possible. Quite a few other architectures do implement
finer-grained access, and even x86 is getting it now, so you may want
to revisit that. The code is already available, and tested, if you
cut that part out of the PaX security patch).

I see a potential bug with PROT_NONE. I'm not sure if it's real, so
could you please confirm? I don't know the Sparc well enough to tell.

In include/asm-sparc64/pgtable.h, there's:

#define __ACCESS_BITS (_PAGE_ACCESSED | _PAGE_READ | _PAGE_R)
#define PAGE_NONE __pgprot (_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_CACHE)
#define PAGE_READONLY __pgprot (_PAGE_PRESENT | _PAGE_VALID | _PAGE_CACHE | \
__ACCESS_BITS)

PAGE_NONE has the hardware _PAGE_PRESENT bit set. However unlike
PAGE_READONLY, it doesn't have the hardware _PAGE_R and software
_PAGE_READ bits.

I guess that means that PAGE_NONE pages aren't readable from
userspace. Presumably the TLB handler takes care of it.
Does it prevent reads from kernel space as well?

I.e. can you confirm that write() won't succeed in reading the data
from a PROT_NONE page on Sparc64? I think that's probably the case.
You'll see why I ask, from the next one:

In include/asm-sparc/pgtsrmmu.h, there's:

#define SRMMU_PAGE_NONE __pgprot(SRMMU_VALID | SRMMU_CACHE | \
SRMMU_PRIV | SRMMU_REF)
#define SRMMU_PAGE_RDONLY __pgprot(SRMMU_VALID | SRMMU_CACHE | \
SRMMU_EXEC | SRMMU_REF)

This one bothers me. The difference is that PROT_NONE pages are not
accessible to userspace, and not executable.

So userspace will get a fault if it tries to read a PROT_NONE page.

But what happens when the kernel reads one? Don't those bits mean
that the read will succeed? I.e. write() on a PROT_NONE page will
succeed, instead of returning EFAULT?

If so, this is a bug. A minor bug, perhaps, but nonetheless I wish to
document it.

I don't know if you would be able to rearrange the pte bits so that a
PROT_NONE page is not accessible to the kernel either. E.g. on i386
this is done by making PROT_NONE not set the "physical" present bit
but a different bit, and "pte_present()" tests both of those bits to
test the virtual present bit.

Alternatively, perhaps in this case simply omitting the SRMMU_REF bit
would be enough? Would that cause the TLB handler to be entered, and
the TLB handler could then refuse access? Again, I don't know enough
about Sparc to say more.

Looking at pgtsun4.h and pgtsun4c.h, I'm not sure about those either.

Thanks,
-- Jamie


2004-06-30 05:17:53

by David Miller

[permalink] [raw]
Subject: Re: A question about PROT_NONE on Sparc and Sparc64

On Wed, 30 Jun 2004 04:05:03 +0100
Jamie Lokier <[email protected]> wrote:

> In include/asm-sparc64/pgtable.h, there's:
>
> #define __ACCESS_BITS (_PAGE_ACCESSED | _PAGE_READ | _PAGE_R)
> #define PAGE_NONE __pgprot (_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_CACHE)
> #define PAGE_READONLY __pgprot (_PAGE_PRESENT | _PAGE_VALID | _PAGE_CACHE | \
> __ACCESS_BITS)
>
> PAGE_NONE has the hardware _PAGE_PRESENT bit set. However unlike
> PAGE_READONLY, it doesn't have the hardware _PAGE_R and software
> _PAGE_READ bits.
>
> I guess that means that PAGE_NONE pages aren't readable from
> userspace. Presumably the TLB handler takes care of it.
> Does it prevent reads from kernel space as well?

Neither user nor kernel can get at that page. If _PAGE_R is not set
we get a real fault no matter who attempts the access.

> I.e. can you confirm that write() won't succeed in reading the data
> from a PROT_NONE page on Sparc64? I think that's probably the case.
> You'll see why I ask, from the next one:

That's correct.

> In include/asm-sparc/pgtsrmmu.h, there's:
>
> #define SRMMU_PAGE_NONE __pgprot(SRMMU_VALID | SRMMU_CACHE | \
> SRMMU_PRIV | SRMMU_REF)
> #define SRMMU_PAGE_RDONLY __pgprot(SRMMU_VALID | SRMMU_CACHE | \
> SRMMU_EXEC | SRMMU_REF)
>
> This one bothers me. The difference is that PROT_NONE pages are not
> accessible to userspace, and not executable.
>
> So userspace will get a fault if it tries to read a PROT_NONE page.
>
> But what happens when the kernel reads one? Don't those bits mean
> that the read will succeed? I.e. write() on a PROT_NONE page will
> succeed, instead of returning EFAULT?
>
> If so, this is a bug. A minor bug, perhaps, but nonetheless I wish to
> document it.

Yes this one is a bug and not intentional.

Keith W., we need to fix this. Probably the simplest fix is just to
drop the SRMMU_VALID bit.

> Alternatively, perhaps in this case simply omitting the SRMMU_REF bit
> would be enough? Would that cause the TLB handler to be entered, and
> the TLB handler could then refuse access? Again, I don't know enough
> about Sparc to say more.

No, if it's SRMMU_VALID the hardware lets the translation succeed and
like on x86 the hardware does the page table walk and thus the SRMMU_REF
bit setting.

2004-06-30 08:28:45

by Jakub Jelinek

[permalink] [raw]
Subject: Re: A question about PROT_NONE on Sparc and Sparc64

On Wed, Jun 30, 2004 at 04:05:03AM +0100, Jamie Lokier wrote:
> I'm doing a survey of the different architectural implementations of
> PROT_* flags for mmap() and mprotect(). I'm looking at linux-2.6.5.
>
> The Sparc and Sparc64 implementations are very similar to plain x86:
> read implies exec, exec implies read and write implies read.
>
> (Aside: A comment in include/asm-sparc/pgtsrmmu.h says that finer-grained
> access is possible. Quite a few other architectures do implement
> finer-grained access, and even x86 is getting it now, so you may want
> to revisit that. The code is already available, and tested, if you
> cut that part out of the PaX security patch).

I believe R!X and X!R pages ought to be possible on sparc64 too, just use a
different bit as "read" in the fast ITLB miss handler from the one fast DTLB
miss uses.

Jakub

2004-06-30 15:21:33

by Keith M Wesolowski

[permalink] [raw]
Subject: Re: A question about PROT_NONE on Sparc and Sparc64

On Tue, Jun 29, 2004 at 10:17:11PM -0700, David S. Miller wrote:

> > In include/asm-sparc/pgtsrmmu.h, there's:
> >
> > #define SRMMU_PAGE_NONE __pgprot(SRMMU_VALID | SRMMU_CACHE | \
> > SRMMU_PRIV | SRMMU_REF)
> > #define SRMMU_PAGE_RDONLY __pgprot(SRMMU_VALID | SRMMU_CACHE | \
> > SRMMU_EXEC | SRMMU_REF)
> >
> > This one bothers me. The difference is that PROT_NONE pages are not
> > accessible to userspace, and not executable.
> >
> > So userspace will get a fault if it tries to read a PROT_NONE page.
> >
> > But what happens when the kernel reads one? Don't those bits mean
> > that the read will succeed? I.e. write() on a PROT_NONE page will
> > succeed, instead of returning EFAULT?
> >
> > If so, this is a bug. A minor bug, perhaps, but nonetheless I wish to
> > document it.
>
> Yes this one is a bug and not intentional.
>
> Keith W., we need to fix this. Probably the simplest fix is just to
> drop the SRMMU_VALID bit.

Ok, I'll try this approach and see what happens.

--
Keith M Wesolowski

2004-06-30 20:55:10

by David Miller

[permalink] [raw]
Subject: Re: A question about PROT_NONE on Sparc and Sparc64

On Wed, 30 Jun 2004 04:28:05 -0400
Jakub Jelinek <[email protected]> wrote:

> I believe R!X and X!R pages ought to be possible on sparc64 too, just use a
> different bit as "read" in the fast ITLB miss handler from the one fast DTLB
> miss uses.

That's correct. But I have no plans to implement this
any time soon :-)

2004-06-30 22:52:29

by Jamie Lokier

[permalink] [raw]
Subject: Re: A question about PROT_NONE on Sparc and Sparc64

David S. Miller wrote:
> On Wed, 30 Jun 2004 04:28:05 -0400
> Jakub Jelinek <[email protected]> wrote:
>
> > I believe R!X and X!R pages ought to be possible on sparc64 too,
> > just use a different bit as "read" in the fast ITLB miss handler
> > from the one fast DTLB miss uses.
>
> That's correct. But I have no plans to implement this
> any time soon :-)

The PaX security patch already implements R!X pages on Sparc64, so you
could just cut out that part of the patch. Just pick out the changes
to arch/sparc64/* and include/asm-sparc64/*:

http://pax.grsecurity.net/pax-linux-2.6.7-200406252135.patch

It appears to use exactly the technique Jakub describes, and has been tested.

-- Jamie