[PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[Wu, Bryan]

Hi folks,

As struct mm_struct vm_mm is hidden in struct vm_area_struct in NOMMU
arch, this is a fixing method when compiling failure on blackfin arch.

Signed-off-by: Bryan Wu <bryan.wu@analog.com> 
---

 fs/revoke.c |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

Index: linux-2.6/fs/revoke.c
===================================================================
--- linux-2.6.orig/fs/revoke.c
+++ linux-2.6/fs/revoke.c
@@ -207,13 +207,21 @@
 /*
  *	LOCKING: spin_lock(&mapping->i_mmap_lock)
  */
-static int revoke_mm(struct mm_struct *mm, struct address_space *mapping,
+static int revoke_mm(struct vm_area_struct *vma, struct address_space *mapping,
 		     struct file *to_exclude)
 {
-	struct vm_area_struct *vma;
+#ifdef CONFIG_MMU
+	struct mm_struct *mm = vma->vm_mm;
+#else
+	struct mm_struct *mm = 0;
+#endif
+	struct vm_area_struct *_vma;
 	struct zap_details details;
 	int err = 0;
 
+	if (!mm)
+		return -ENOENT;
+
 	details.i_mmap_lock = &mapping->i_mmap_lock;
 
 	/*
@@ -224,11 +232,11 @@
 		err = -EAGAIN;
 		goto out;
 	}
-	for (vma = mm->mmap; vma != NULL; vma = vma->vm_next) {
-		if (!need_revoke(vma, to_exclude))
+	for (_vma = mm->mmap; _vma != NULL; _vma = _vma->vm_next) {
+		if (!need_revoke(_vma, to_exclude))
 			continue;
 
-		err = revoke_vma(vma, &details);
+		err = revoke_vma(_vma, &details);
 		if (err)
 			break;
 	}
@@ -254,7 +262,7 @@
 		if (likely(!need_revoke(vma, to_exclude)))
 			continue;
 
-		err = revoke_mm(vma->vm_mm, mapping, to_exclude);
+		err = revoke_mm(vma, mapping, to_exclude);
 		if (err == -EAGAIN) {
 			try_again = 1;
 			continue;
@@ -284,7 +292,7 @@
 		if (likely(!need_revoke(vma, to_exclude)))
 			continue;
 
-		err = revoke_mm(vma->vm_mm, mapping, to_exclude);
+		err = revoke_mm(vma, mapping, to_exclude);
 		if (err == -EAGAIN) {
 			try_again = 1;
 			continue;
_

Thanks,
-Bryan Wu

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[Pekka J Enberg]

On Mon, 26 Mar 2007, Wu, Bryan wrote:
> As struct mm_struct vm_mm is hidden in struct vm_area_struct in NOMMU
> arch, this is a fixing method when compiling failure on blackfin arch.

What compile error is that? I don't see any #ifdef around ->vm_mm for 
struct vm_area_struct in <linux/mm.h>.

On Mon, 26 Mar 2007, Wu, Bryan wrote:
> +	if (!mm)
> +		return -ENOENT;
> +
>  	details.i_mmap_lock = &mapping->i_mmap_lock;

This means you won't be able to revoke memory mapped files with no-MMU. 
I'd rather fix it properly if someone can give me a clue how mmap works on 
non-MMU.

			Pekka

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[Andrew Morton]

On Mon, 26 Mar 2007 18:23:57 +0800 "Wu, Bryan" <bryan.wu@analog.com> wrote:

> Hi folks,
> 
> As struct mm_struct vm_mm is hidden in struct vm_area_struct in NOMMU
> arch, this is a fixing method when compiling failure on blackfin arch.
> 
> Signed-off-by: Bryan Wu <bryan.wu@analog.com> 
> ---
> 
>  fs/revoke.c |   22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> Index: linux-2.6/fs/revoke.c
> ===================================================================
> --- linux-2.6.orig/fs/revoke.c
> +++ linux-2.6/fs/revoke.c
> @@ -207,13 +207,21 @@
>  /*
>   *	LOCKING: spin_lock(&mapping->i_mmap_lock)
>   */
> -static int revoke_mm(struct mm_struct *mm, struct address_space *mapping,
> +static int revoke_mm(struct vm_area_struct *vma, struct address_space *mapping,
>  		     struct file *to_exclude)
>  {
> -	struct vm_area_struct *vma;
> +#ifdef CONFIG_MMU
> +	struct mm_struct *mm = vma->vm_mm;
> +#else
> +	struct mm_struct *mm = 0;
> +#endif

<looks>

Offending patch is
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc4/2.6.21-rc4-mm1/broken-out/nommu-hide-vm_mm-in-nommu-mode.patch,
which seems rather dumb.  Or at least, its changelog does a good job of
making it look dumb.

David, what on earth does "this isn't used there" mean?  Surely it is
logical to have the mm backpointer in the vma in nommu mode?  What's going
on here?

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[David Howells]

Andrew Morton <akpm@linux-foundation.org> wrote:

> Offending patch is
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc4/2.6.21-rc4-mm1/broken-out/nommu-hide-vm_mm-in-nommu-mode.patch,
> which seems rather dumb.  Or at least, its changelog does a good job of
> making it look dumb.

vm_mm is always NULL under NOMMU as it currently stands.  As far as I know,
this has been true since the NOMMU mm stuff was first included (I'm not sure
the VMAs of the first NOMMU mm code *had* a vm_mm).  Hugh (I think it was)
suggested that since this was always NULL, then it should be excised from the
struct in NOMMU-mode.

The reason is that, at the moment, VMAs are a global *shared* resource in
NOMMU-mode.  Each process has a list of global VMAs that it subscribes to, but
that's it.  This (a) slightly reduces the amount of metadata allocated
(possibly), and (b) makes sharing of executables and libraries much easier.

I have started on a patch to make VMAs non-shared, but it's not trivial.  In
NOMMU-mode, there needs to be something to pin the memory a private mapping
uses that isn't required in MMU-mode.  In the shared VMA thing, the VMA itself
can pin that memory...

I wonder if revoke_mm() is something that you can't do in NOMMU-mode.  What
does it do?

David

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[Pekka J Enberg]

Hi David,

On Mon, 26 Mar 2007, David Howells wrote:
> The reason is that, at the moment, VMAs are a global *shared* resource in
> NOMMU-mode.  Each process has a list of global VMAs that it subscribes to, but
> that's it.  This (a) slightly reduces the amount of metadata allocated
> (possibly), and (b) makes sharing of executables and libraries much easier.

On Mon, 26 Mar 2007, David Howells wrote:
> I wonder if revoke_mm() is something that you can't do in NOMMU-mode.  What
> does it do?

The revoke_mm() function scans all vmas of a mm and revokes those that 
are shared and point to the inode being revoked. So, NOMMU can't do that.

But what's more important is, can we do revoke_mapping() for NOMMU? AFAICT 
we can, we just need to scan all the global vmas, right?

				Pekka

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[David Howells]

Pekka J Enberg <penberg@cs.helsinki.fi> wrote:

> But what's more important is, can we do revoke_mapping() for NOMMU? AFAICT 
> we can, we just need to scan all the global vmas, right?

I don't know, what does it do?  Remember, once a NOMMU process thinks it has
the right to access a mapping, there's no way of stopping it doing so short of
killing the process.

With NOMMU as it stands, private mappings are private copies of the data, and
have no impact on the page cache and get no updates from it.  It's as if you
took a private writable mapping, touched every page and then mprotect()'d it.
This isn't necessarily ideal, but we're limited by the lack on an MMU.

David

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[Andrew Morton]

On Mon, 26 Mar 2007 12:25:18 +0100 David Howells <dhowells@redhat.com> wrote:

> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > Offending patch is
> > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc4/2.6.21-rc4-mm1/broken-out/nommu-hide-vm_mm-in-nommu-mode.patch,
> > which seems rather dumb.  Or at least, its changelog does a good job of
> > making it look dumb.
> 
> vm_mm is always NULL under NOMMU as it currently stands.  As far as I know,
> this has been true since the NOMMU mm stuff was first included (I'm not sure
> the VMAs of the first NOMMU mm code *had* a vm_mm).  Hugh (I think it was)
> suggested that since this was always NULL, then it should be excised from the
> struct in NOMMU-mode.
> 
> The reason is that, at the moment, VMAs are a global *shared* resource in
> NOMMU-mode.  Each process has a list of global VMAs that it subscribes to, but
> that's it.  This (a) slightly reduces the amount of metadata allocated
> (possibly), and (b) makes sharing of executables and libraries much easier.

whoa.  You live and learn.  Logical, I guess.

I agree that in that case, we just don't want vm_mm to exist in NOMMU builds - it's
better to fail at compile time.

I'll touch up the changelog for nommu-hide-vm_mm-in-nommu-mode.patch and then
I'll temporarily drop it so the blackfin guys can test their work, I guess.

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[Pekka J Enberg]

On Mon, 26 Mar 2007, David Howells wrote:
> I don't know, what does it do?  Remember, once a NOMMU process thinks it has
> the right to access a mapping, there's no way of stopping it doing so short of
> killing the process.

revoke_mapping() is mostly same as munmap(2) except that it preserves the 
vma but makes it VM_REVOKED. This means that if the process tries to 
access the region it will SIGBUS and if it tries to remap the range it 
will get EINVAL.

What we're trying to do here is, we want to make sure no other tasks can 
access the inode once it has been revoked.

On Mon, 26 Mar 2007, David Howells wrote:
> With NOMMU as it stands, private mappings are private copies of the data, and
> have no impact on the page cache and get no updates from it.  It's as if you
> took a private writable mapping, touched every page and then mprotect()'d it.
> This isn't necessarily ideal, but we're limited by the lack on an MMU.

So there's no way to raise SIGBUS if the range is being accessed. The 
alternatives are:

  - No support for revoke(2) on NOMMU.
  - If there are shared mappings, always return -ENOENT for revoke(2).
  - If there are shared mappings, immediately raise SIGBUS for those 
    processes that are accessing it.

Making the shared mappings private is not an option because there's no way 
for the process to know that it's mapping is being pulled under it which 
will result in bugs. Hmm.

				Pekka

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[David Howells]

Andrew Morton <akpm@linux-foundation.org> wrote:

> I'll touch up the changelog for nommu-hide-vm_mm-in-nommu-mode.patch and then
> I'll temporarily drop it so the blackfin guys can test their work, I guess.

Thanks.

As I said, I'm also not sure that revocation of VMAs is supportable on NOMMU,
so the thing to do may be to hide it entirely if CONFIG_MMU=n.

David

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[David Howells]

Pekka J Enberg <penberg@cs.helsinki.fi> wrote:

> > I don't know, what does it do?  Remember, once a NOMMU process thinks it
> > has the right to access a mapping, there's no way of stopping it doing so
> > short of killing the process.
> 
> revoke_mapping() is mostly same as munmap(2) except that it preserves the 
> vma but makes it VM_REVOKED. This means that if the process tries to 
> access the region it will SIGBUS and if it tries to remap the range it 
> will get EINVAL.

Yeah, that's not enforceable in NOMMU-mode situations.  I presume it differs
from munmap() also in that it can effectively be forced by one process upon
another.

In MMU-mode, how does this work with private mappings that have some private
copies of the pages that make up the mapping?  Are those still available to a
process that is using them?  Are they revoked when swapped out?  Or are they
forcibly evicted?

> What we're trying to do here is, we want to make sure no other tasks can 
> access the inode once it has been revoked.

Okay.

> So there's no way to raise SIGBUS if the range is being accessed. The 
> alternatives are:
> 
>   - No support for revoke(2) on NOMMU.

That's a bit over the top, I think.  It sounds like revoke() is perfectly fine
- as long as there aren't any mappings on the target inode (or at least shared
mappings - dunno about private mappings).

>   - If there are shared mappings, always return -ENOENT for revoke(2).

That sounds feasible.  How about -ETXTBSY instead?

>   - If there are shared mappings, immediately raise SIGBUS for those 
>     processes that are accessing it.

Hmmm... maybe.  That sounds a bit antisocial though, but is also workable.
Does the SIGBUS raised have its own si_code, btw?  Perhaps BUS_REVOKED?

> Making the shared mappings private is not an option because there's no way 
> for the process to know that it's mapping is being pulled under it which 
> will result in bugs. Hmm.

Agreed.

David

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[Pekka J Enberg]

Hi,

Pekka J Enberg <penberg@cs.helsinki.fi> wrote:
> > revoke_mapping() is mostly same as munmap(2) except that it preserves the 
> > vma but makes it VM_REVOKED. This means that if the process tries to 
> > access the region it will SIGBUS and if it tries to remap the range it 
> > will get EINVAL.

On Mon, 26 Mar 2007, David Howells wrote:
> Yeah, that's not enforceable in NOMMU-mode situations.  I presume it differs
> from munmap() also in that it can effectively be forced by one process upon
> another.

Yes.

On Mon, 26 Mar 2007, David Howells wrote:
> In MMU-mode, how does this work with private mappings that have some private
> copies of the pages that make up the mapping?  Are those still available to a
> process that is using them?  Are they revoked when swapped out?  Or are they
> forcibly evicted?

We don't touch private mappings at all as they're a snapshot to the inode 
_before_ it was revoked. So private mappings don't really matter all: you 
don't see any new data after it has been revoked nor do you flush anything 
to the disk.

Pekka J Enberg <penberg@cs.helsinki.fi> wrote:
> >   - If there are shared mappings, always return -ENOENT for revoke(2).
 
On Mon, 26 Mar 2007, David Howells wrote:
> That sounds feasible.  How about -ETXTBSY instead?

Well, assuming we would use revoke for things like SAK, this doesn't 
really work out too well because all a malicious process has to is create 
a shared mapping and they've effectively blocked the whole thing.

Pekka J Enberg <penberg@cs.helsinki.fi> wrote:
> >   - If there are shared mappings, immediately raise SIGBUS for those 
> >     processes that are accessing it.

On Mon, 26 Mar 2007, David Howells wrote:
> Hmmm... maybe.  That sounds a bit antisocial though, but is also 
> workable.

It's antisocial for sure but the only way to guarantee revoke() succeeds 
on a NOMMU setup. Oh well, lets disable it for now and see if anyone even 
wants revoke() for NOMMU.

Pekka J Enberg <penberg@cs.helsinki.fi> wrote:
> Does the SIGBUS raised have its own si_code, btw?  Perhaps BUS_REVOKED?

That's a good idea. I'll add one.

			Pekka

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[Alan Cox]

> With NOMMU as it stands, private mappings are private copies of the data, and
> have no impact on the page cache and get no updates from it.  It's as if you
> took a private writable mapping, touched every page and then mprotect()'d it.
> This isn't necessarily ideal, but we're limited by the lack on an MMU.

Given the MMUless kernel has no security model the easiest is probably to
simply not support revoke() of mmap areas on NOMMU (or maybe not to
support revoke at all)

Alan

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[David Howells]

Pekka J Enberg <penberg@cs.helsinki.fi> wrote:

> We don't touch private mappings at all as they're a snapshot to the inode 
> _before_ it was revoked. So private mappings don't really matter all: you 
> don't see any new data after it has been revoked nor do you flush anything 
> to the disk.

Okay, so that's not a problem.

> Well, assuming we would use revoke for things like SAK, this doesn't 
> really work out too well because all a malicious process has to is create 
> a shared mapping and they've effectively blocked the whole thing.

In NOMMU-mode, there's probably[*] nothing stopping a malicious process
running completely amok and changing stuff directly - even the kernel isn't
guaranteed to be safe - so I wouldn't worry about such a case.

[*] The FRV, for example, does have some limited protection capability - but
it is really limited and not really useful in this case.

> It's antisocial for sure but the only way to guarantee revoke() succeeds on
> a NOMMU setup.  Oh well, lets disable it for now and see if anyone even
> wants revoke() for NOMMU.

Agreed.

David

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[Mike Frysinger]

On 3/26/07, David Howells <dhowells@redhat.com> wrote:
> [*] The FRV, for example, does have some limited protection capability - but
> it is really limited and not really useful in this case.

how so ?  the Blackfin processor lacks a MMU but it does have a MPU
(memory protection unit) which allows granularity down to 1k page
sizes ... so for future releases, we plan on integrating optional
support for this so that you could have processes protected from each
other and the kernel protected from all the processes ... so in our
case, we might actually be able to support revoking of maps because we
would have that region of memory ear marked as unaccessible ...

note that the Blackfin processor manuals confusingly call this aspect
of the chip an "MMU" ... dont be fooled !
-mike

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[Wu, Bryan]

On Mon, 2007-03-26 at 16:21 -0400, Mike Frysinger wrote:
> On 3/26/07, David Howells <dhowells@redhat.com> wrote:
> > [*] The FRV, for example, does have some limited protection capability - but
> > it is really limited and not really useful in this case.
> 

Sorry for late response.

> how so ?  the Blackfin processor lacks a MMU but it does have a MPU
> (memory protection unit) which allows granularity down to 1k page
> sizes ... so for future releases, we plan on integrating optional
> support for this so that you could have processes protected from each
> other and the kernel protected from all the processes ... so in our
> case, we might actually be able to support revoking of maps because we
> would have that region of memory ear marked as unaccessible ...
> 

Agree. MPU of Blackfin can provide some processes protection. But maybe
at this moment just disable revoke for NOMMU is easier for further
development. When we provide the MPU stuff, maybe we can enable the
revoke for NOMMU but MPU arch.

> note that the Blackfin processor manuals confusingly call this aspect
> of the chip an "MMU" ... dont be fooled !
> -mike

Thanks
-Bryan

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[Pekka J Enberg]

On Tue, 27 Mar 2007, Wu, Bryan wrote:
> Agree. MPU of Blackfin can provide some processes protection. But maybe
> at this moment just disable revoke for NOMMU is easier for further
> development. When we provide the MPU stuff, maybe we can enable the
> revoke for NOMMU but MPU arch.

Yeah, MPU should be sufficient for revoke(). Note that mmap() and mummap() 
suffers from the same problem on NOMMU so perhaps we really should do 
CONFIG_MPU.

				Pekka

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[Mike Frysinger]

On 3/27/07, Pekka J Enberg <penberg@cs.helsinki.fi> wrote:
> On Tue, 27 Mar 2007, Wu, Bryan wrote:
> > Agree. MPU of Blackfin can provide some processes protection. But maybe
> > at this moment just disable revoke for NOMMU is easier for further
> > development. When we provide the MPU stuff, maybe we can enable the
> > revoke for NOMMU but MPU arch.
>
> Yeah, MPU should be sufficient for revoke(). Note that mmap() and mummap()
> suffers from the same problem on NOMMU so perhaps we really should do
> CONFIG_MPU.

we would need both ... MPU gives us the ability to restrict access,
but we'd still be unable to do fun things like COW
-mike

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[David Howells]

Mike Frysinger <vapier.adi@gmail.com> wrote:

> > [*] The FRV, for example, does have some limited protection capability - but
> > it is really limited and not really useful in this case.
> 
> how so ?

There are a limited set of protection registers (At least 8 insn and 8 data)
that can permit tiles of protection that are a power-of-2 megabytes in size.
However, these also have to be used to grant the kernel access to h/w
(including RAM).

Note that it's not possible to shift windows around in response to faults
because fault reporting is asynchronous - the entire remaining instruction
queue will be executed *before* the exception is actually raised to the
kernel.

> the Blackfin processor lacks a MMU but it does have a MPU (memory protection
> unit) which allows granularity down to 1k page sizes ... so for future
> releases, we plan on integrating optional support for this so that you could
> have processes protected from each other and the kernel protected from all
> the processes ... so in our case, we might actually be able to support
> revoking of maps because we would have that region of memory ear marked as
> unaccessible ...

That sounds reasonable.  However, I suspect that most NOMMU CPUs won't be able
to do that.  In effect you're creating a third option, I think (MMU, NOMMU,
MPU).

> note that the Blackfin processor manuals confusingly call this aspect
> of the chip an "MMU" ... dont be fooled !

Same for FRV.  It is memory management - it just doesn't include virtual
memory mapping.

David

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[Mike Frysinger]

On 3/27/07, David Howells <dhowells@redhat.com> wrote:
> Note that it's not possible to shift windows around in response to faults
> because fault reporting is asynchronous - the entire remaining instruction
> queue will be executed *before* the exception is actually raised to the
> kernel.

ah that would limit the usefulness of its application in the kernel
... thanks for the info

> That sounds reasonable.  However, I suspect that most NOMMU CPUs won't be able
> to do that.  In effect you're creating a third option, I think (MMU, NOMMU,
> MPU).

sure, but i'm not sure the MPU option would be mutually exclusive with
NOMMU ... in the Blackfin case, we'd want both ...
-mike

Re: [PATCH -mm] Revoke core code: fix nommu arch compiling error bug

[David Howells]

Mike Frysinger <vapier.adi@gmail.com> wrote:

> > That sounds reasonable.  However, I suspect that most NOMMU CPUs won't be
> > able to do that.  In effect you're creating a third option, I think (MMU,
> > NOMMU, MPU).
> 
> sure, but i'm not sure the MPU option would be mutually exclusive with
> NOMMU ... in the Blackfin case, we'd want both ...

I imagine that MPU would be a superset of NOMMU.

David