2009-07-09 08:05:29

by Mike Frysinger

[permalink] [raw]
Subject: truncate on MAP_SHARED files in ramfs filesystems on no-mmu

reviewing LTP tests shows mmap09 failing. this test creates a file of
a certain length, opens it and creates a shared mapping, and then
tries various truncate tests:
truncate to a smaller size
truncate to a larger size
truncate to size 0

the first and last fail on no-mmu due to
file-nommu.c:ramfs_nommu_resize() rejecting attempts to shrink on a
shared mapping:
/* check that a decrease in size doesn't cut off any shared mappings */
if (newsize < size) {
ret = ramfs_nommu_check_mappings(inode, newsize, size);
if (ret < 0)
return ret;
}

my question is why ? if an application maps a fd with MAP_SHARED,
truncates it, and then it or someone else who has that fd mapped tries
to access the now-invalid tail end, that is a bug in the application.
i dont see why we should be protecting users here from their own buggy
code ?
-mike


2009-07-09 10:07:15

by David Howells

[permalink] [raw]
Subject: Re: truncate on MAP_SHARED files in ramfs filesystems on no-mmu

Mike Frysinger <[email protected]> wrote:

> reviewing LTP tests shows mmap09 failing. this test creates a file of
> a certain length, opens it and creates a shared mapping, and then
> tries various truncate tests:
> truncate to a smaller size
> truncate to a larger size
> truncate to size 0
>
> the first and last fail on no-mmu due to
> file-nommu.c:ramfs_nommu_resize() rejecting attempts to shrink on a
> shared mapping:

Yes. That's exactly right.

> my question is why? if an application maps a fd with MAP_SHARED,
> truncates it, and then it or someone else who has that fd mapped tries
> to access the now-invalid tail end, that is a bug in the application.
> i dont see why we should be protecting users here from their own buggy
> code ?

This is protecting the kernel as much as the user. There's no MMU to enforce
denial of access on the pages that truncate returns to the system.

This doesn't only protect the process with a mapping on that file against its
own truncate, but also other processes that have made mappings against that
file.

Whilst you can argue it either way, you need a better reason to change this
than it causes some LTP failures. You cannot expect all the MM-related LTP
tests to work against a NOMMU system.

Doing it this way also makes things simpler in the kernel and makes the system
more robust.

If a process shared mmaps a file and then wants to truncate it, it can always
munmap the excess first.

David

2009-07-09 15:22:32

by Mike Frysinger

[permalink] [raw]
Subject: Re: truncate on MAP_SHARED files in ramfs filesystems on no-mmu

On Thu, Jul 9, 2009 at 06:06, David Howells wrote:
> Mike Frysinger wrote:
>> reviewing LTP tests shows mmap09 failing.  this test creates a file of
>> a certain length, opens it and creates a shared mapping, and then
>> tries various truncate tests:
>> truncate to a smaller size
>> truncate to a larger size
>> truncate to size 0
>>
>> the first and last fail on no-mmu due to
>> file-nommu.c:ramfs_nommu_resize() rejecting attempts to shrink on a
>> shared mapping:
>
> Yes.  That's exactly right.
>
>> my question is why?  if an application maps a fd with MAP_SHARED,
>> truncates it, and then it or someone else who has that fd mapped tries
>> to access the now-invalid tail end, that is a bug in the application.
>> i dont see why we should be protecting users here from their own buggy
>> code ?
>
> This is protecting the kernel as much as the user.  There's no MMU to enforce
> denial of access on the pages that truncate returns to the system.

you dont need a MMU (virtual memory) to protect against it. you only
need a MPU which some systems have.

> This doesn't only protect the process with a mapping on that file against its
> own truncate, but also other processes that have made mappings against that
> file.

and those too are broken

> Whilst you can argue it either way, you need a better reason to change this
> than it causes some LTP failures.  You cannot expect all the MM-related LTP
> tests to work against a NOMMU system.

crappy programming is likely to crash regardless of standard functions
we attempt to disable in the kernel. this isnt a virtual memory issue
at all, it's memory protection.

> Doing it this way also makes things simpler in the kernel and makes the system
> more robust.

really ? looks like the kernel is a lot more complicated to me. the
fix here would be to delete a whole bunch of code.

> If a process shared mmaps a file and then wants to truncate it, it can always
> munmap the excess first.

sure, we could go around changing a whole bunch of things specific to
no-mmu, but that's kind of the wrong way to go. applications shouldnt
need to know they're running with different MMU features available.
-mike

2009-07-09 16:07:23

by David Howells

[permalink] [raw]
Subject: Re: truncate on MAP_SHARED files in ramfs filesystems on no-mmu

Mike Frysinger <[email protected]> wrote:

> you dont need a MMU (virtual memory) to protect against it. you only
> need a MPU which some systems have.

You may not have that either. FRV doesn't, for example. Furthermore, if you
have an MPU only, you can still do a lot of the missing bits of NOMMU mmap() -
shared writable disk or NFS files for example, so it can be argued that
MPU-only systems shouldn't be using mm/nommu.c.

> > This doesn't only protect the process with a mapping on that file against
> > its own truncate, but also other processes that have made mappings against
> > that file.
>
> and those too are broken

Not necessarily. They may not be expecting the truncation. Just because the
first process might be incorrect doesn't mean that the other affected processes
are.

> > Whilst you can argue it either way, you need a better reason to change this
> > than it causes some LTP failures. You cannot expect all the MM-related LTP
> > tests to work against a NOMMU system.
>
> crappy programming is likely to crash regardless of standard functions we
> attempt to disable in the kernel. this isnt a virtual memory issue at all,
> it's memory protection.

Are you actually seeing this in a real world situation? Or just in LTP?

> > Doing it this way also makes things simpler in the kernel and makes the
> > system more robust.
>
> really? looks like the kernel is a lot more complicated to me. the fix here
> would be to delete a whole bunch of code.

Delete what? The check for ramfs_nommu_check_mappings()? That is not
sufficient. That might allow truncate to give the pages back to the system,
but the pages are still pointed to by VMAs and regions. NOMMU truncate, as it
stands, will not take care of that: unmap_mapping_range() is not implemented
for NOMMU as the aforementioned check renders it unnecessary.

It is simpler in that we simply reject a truncate that would cut down a mapping
rather than trying to shrink that mapping.

It is more robust in that if one process has a file mapped, and another process
truncates it, then that second process isn't prevented from accessing the
region that has been taken away from it.

> > If a process shared mmaps a file and then wants to truncate it, it can
> > always munmap the excess first.
>
> sure, we could go around changing a whole bunch of things specific to no-mmu,
> but that's kind of the wrong way to go. applications shouldnt need to know
> they're running with different MMU features available.

Can you point to a real world case where this is a problem?


Note that it would be very easy to add (if such does not already exist) an LTP
test that creates a file, expands it, maps it, shrinks it and then attempts to
alter the removed part of the mapping in the expectation of receiving a SIGBUS.

As it stands, such a test will work on MMU, but go wrong on NOMMU in a
different way in these two cases. With the current behaviour, the shrink
request will be rejected, but the system will survive. With your proposed
behaviour, the system will potentially be wrecked.

The NOMMU situation behaves differently to the MMU situation in either case.

David

2009-07-10 16:53:00

by Robin Getz

[permalink] [raw]
Subject: Re: truncate on MAP_SHARED files in ramfs filesystems on no-mmu

On Thu 9 Jul 2009 12:07, David Howells pondered:
> Note that it would be very easy to add (if such does not already exist)
> an LTP test that creates a file, expands it, maps it, shrinks it and then
> attempts to alter the removed part of the mapping in the expectation
> of receiving a SIGBUS.
>
> As it stands, such a test will work on MMU, but go wrong on NOMMU in a
> different way in these two cases. With the current behaviour, the
> shrink request will be rejected, but the system will survive. With your
> proposed behaviour, the system will potentially be wrecked.
>
> The NOMMU situation behaves differently to the MMU situation in either
> case.

I guess what we were looking at - was the normal case of when userspace
doesn't go wrong.

The kernel (today in noMMU) assumes the userspace is going to do something bad
after it does the shrink - but for those people who are using things
properly - it doesn't allow it to work at all.

2009-07-10 19:35:20

by Mike Frysinger

[permalink] [raw]
Subject: Re: truncate on MAP_SHARED files in ramfs filesystems on no-mmu

On Thu, Jul 9, 2009 at 12:07, David Howells wrote:
> Mike Frysinger wrote:
>> you dont need a MMU (virtual memory) to protect against it.  you only
>> need a MPU which some systems have.
>
> You may not have that either.  FRV doesn't, for example.

i wasnt suggesting every no-mmu architecture had one. hence the word "some".

> Furthermore, if you
> have an MPU only, you can still do a lot of the missing bits of NOMMU mmap() -
> shared writable disk or NFS files for example, so it can be argued that
> MPU-only systems shouldn't be using mm/nommu.c.

perhaps, but the mmu code cant be used without virtual memory, and we
havent reviewed all the different aspects of the nommu code which
should be split based on MPU availability. we have a patch locally
that i should push for the next release that adds appropriate calls to
the protection functions in kernel/module.c and mm/nommu.c. basically
enough to get us up and running with standard rwx markings.

>> > This doesn't only protect the process with a mapping on that file against
>> > its own truncate, but also other processes that have made mappings against
>> > that file.
>>
>> and those too are broken
>
> Not necessarily.  They may not be expecting the truncation.  Just because the
> first process might be incorrect doesn't mean that the other affected processes
> are.

you are correct, but in the end it's largely the same -- there is a
bug in userspace here that someone needs to go fix

>> > Whilst you can argue it either way, you need a better reason to change this
>> > than it causes some LTP failures.  You cannot expect all the MM-related LTP
>> > tests to work against a NOMMU system.
>>
>> crappy programming is likely to crash regardless of standard functions we
>> attempt to disable in the kernel.  this isnt a virtual memory issue at all,
>> it's memory protection.
>
> Are you actually seeing this in a real world situation?  Or just in LTP?

atm, just LTP. but simply discarding out of hand as "it's an
unrealistic LTP testcase" may not be appropriate. many of the
testcases in LTP come from real world experience and tests. i know
many of the tests ive added to LTP werent for fun but stripped down
test cases of real applications failing.

>> > Doing it this way also makes things simpler in the kernel and makes the
>> > system more robust.
>>
>> really?  looks like the kernel is a lot more complicated to me.  the fix here
>> would be to delete a whole bunch of code.
>
> Delete what?  The check for ramfs_nommu_check_mappings()?  That is not
> sufficient.  That might allow truncate to give the pages back to the system,
> but the pages are still pointed to by VMAs and regions.  NOMMU truncate, as it
> stands, will not take care of that: unmap_mapping_range() is not implemented
> for NOMMU as the aforementioned check renders it unnecessary.

so we need to first fix the nommu vmtruncate function so that it
actually updates the VMAs ?

> It is simpler in that we simply reject a truncate that would cut down a mapping
> rather than trying to shrink that mapping.
>
> It is more robust in that if one process has a file mapped, and another process
> truncates it, then that second process isn't prevented from accessing the
> region that has been taken away from it.

it is also different behavior from mmu (i dont know what POSIX has to
say on using truncate on a shared mmap -- this is kind of an edge
case). we aim to reduce functional differences at the kernel level
rather than attempting to change behavior of every application we come
across.

>> > If a process shared mmaps a file and then wants to truncate it, it can
>> > always munmap the excess first.
>>
>> sure, we could go around changing a whole bunch of things specific to no-mmu,
>> but that's kind of the wrong way to go.  applications shouldnt need to know
>> they're running with different MMU features available.
>
> Can you point to a real world case where this is a problem?
>
>
> Note that it would be very easy to add (if such does not already exist) an LTP
> test that creates a file, expands it, maps it, shrinks it and then attempts to
> alter the removed part of the mapping in the expectation of receiving a SIGBUS.
>
> As it stands, such a test will work on MMU, but go wrong on NOMMU in a
> different way in these two cases.  With the current behaviour, the shrink
> request will be rejected, but the system will survive.  With your proposed
> behaviour, the system will potentially be wrecked.

the behavior would be different, but now you're comparing two
different things. in the first case (truncating a shared mapping),
all nommu hardware can support this (well, enlarging a mapping may
fail if the memory right after it is not available, but this could
easily happen on a mmu system too). in the second case, nommu
hardware that has a MPU unit would function the same as the mmu port,
but LTP can (and does) track tests that require virtual memory or
memory protection. this test in question requires neither.
-mike