Re: mprotect() resets caching policy

[Hugh Dickins <hugh@veritas.com>]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2880 bytes --]

On Fri, 27 Jan 2006, Thomas Hellström wrote:
> 
> I'm working on an infrastructure to allow drm clients to flip arbitrary pages
> in and out of the AGP aperture (or any similar device). In order to avoid
> conflicting mappings for those pages, the caching attribute of both the kernel
> mapping and all VMA's is changed when binding / unbinding.

I can't comment on that plan.

> However, I noticed that mprotect() will, when run on a non-cached VMA, reset
> the caching policy. The line in mm/mprotect.c causing this problem is
> 
> newprot = protection_map[newflags & 0xf];
> 
> So a user could potentially run mprotect() and create a conflicting mapping
> which presumably is bad for stability on some architectures.

Perhaps: I think it already depends on what the architecture does.
newprot is used (a) to set vm_page_prot and (b) for use in pte_modify
(which change_protection applies to each pte present).

Now I think vm_page_prot is irrelevant to the kinds of VMA you are
interested in?  It's essential to provide the permissions/protections
when faulting a new page in, but your VMAs are fully managed by the
driver, and have all ptes already in place?  So wouldn't use vm_page_prot.

And pte_modify is implemented per-architecture: looking just at the
i386 implementation, yes, _PAGE_CHG_MASK looks like it'll mask off
the bits you understandably want it to retain.

> Since mprotect() only deals with rwx protection. I figure replacing the above
> with something like
> 
> newprot = (vm_page_prot & ~MPROT_MASK) | (protection_map[newflags & 0xf] &
> MPROT_MASK)
> 
> Where MPROT_MASK is a arch-dependent mask identifying the bits available to
> mprotect().

I think it's the per-architecture implementations of pte_modify that
you need to adjust.

It might be nice, but probably irrelevant, to have vm_page_prot maintained
in a similar way.  Whether every arch can do that with a straightforward
MPROT_MASK or _PAGE_CHG_MASK is not obvious to me: would more likely
need a pte_modify-like macro to do it.

But would that even be correct?  The same vm_page_prot would be applied
also to anonymous COWed pages from the mapping.  Very exceptional in the
case that interests you; but perhaps it's simpler to keep vm_page_prot
just for the rwx part of it.

> Alternatively, is there a way to disable mprotect() for a VMA?

Not at present.  It would be easier to add a VM_flag for that,
than to correct every architecture's pte_modify.  I'm not sure
whether "easier" amounts to "better" here.

> Finally, is there a chance to get protection_map[] exported to modules?

I very much doubt that.  Exporting a functional interface to it would
be preferable; but even that, I think the core mm would strongly resist
- the less of the pte business we export the better.  What were you
wanting it exported for?

Hugh