Nick Piggin wrote:
On Thu, Nov 29, 2007 at 02:45:23PM -0500, Chuck Ebbert wrote:
Original report: https://bugzilla.redhat.com/show_bug.cgi?id=404201
The test case below, taken from the LTP test code, prints -1 (as
expected) on 2.6.22 and 0 on 2.6.23. It tries to remap an out-of-range
page. Proposed patch follows the program. Bug was apparently caused by
commit 54cb8821de07f2ffcd28c380ce9b93d5784b40d7.
Ah, that's not such good behaviour anyway. mmap is allowed to map
outside the file offset, so you're telling me that remap_file_pages
just magically should not be allowed to remap these...?
Validation check for pgoff was there in populate() in earlier
kernels.When populate() got removed and populate_range() was added,
during the specified commit, validation for pgoff also got removed. This
symantic would break existing apps that expects an error from
remap_file_pages when a large value for pgoff is given. Though the
change is error handling related, it breaks ABI from previous kernel
versions.
For validation, we check whether the pgoff + size exceeds the file size,
all in page units. And while calculating file size in page units, one
additional page unit is taken into account to get the exact number of
pages that contain the file size in bytes.
f_size = i_size_read(mapping->host) + PAGE_CACHE_SIZE - 1;
<---- file size in bytes -------> <--- helps in rounding to next page
unit -->
mmap() will be mapping the minimum number of pages that can contain a
file. So offset cannot be a large value compared to file size. mmap() is
also supposed to return EINVAL when the offset is a large/invalid value
as man page mandates.
Patch:
Signed-off-by: Supriya Kannery <[email protected]>
--- linux-2.6.23/mm/fremap.c.orig 2007-11-22 00:56:09.000000000 -0600
+++ linux-2.6.23/mm/fremap.c 2007-11-26 03:08:55.000000000 -0600
@@ -124,6 +124,7 @@ asmlinkage long sys_remap_file_pages(uns
struct vm_area_struct *vma;
int err = -EINVAL;
int has_write_lock = 0;
+ unsigned long f_size = 0;
if (__prot)
return err;
@@ -181,6 +182,14 @@ asmlinkage long sys_remap_file_pages(uns
goto retry;
}
mapping = vma->vm_file->f_mapping;
+
+ f_size = i_size_read(mapping->host) + PAGE_CACHE_SIZE - 1;
+ f_size = f_size >> PAGE_CACHE_SHIFT;
+ if ((pgoff + size >> PAGE_CACHE_SHIFT) > f_size) {
+ err = -EINVAL;
+ goto out;
+ }
+
/*
* page_mkclean doesn't work on nonlinear vmas, so if
* dirty pages need to be accounted, emulate with linear
I don't think there is anything preventing truncate races here. Theoretically
we could do it by taking i_mutex around here, but anyway then a subsequent
truncate is just going to be able to cause the mapping to be out of bounds
anyway.
i_size_read() is taking care of syncing between the writes/truncations
in SMP/ pre-emtable kernel. For SMP, it specifically takes care to get
the value again if any changes happen to the source.
If it were any other syscall than remap_file_pages, I'd be much more
hesitant to say this: I propose we change the test case instead. I
also changed other elements of the API, and we had the result tested
and verified by Oracle...
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Thanks, Supriya
Linux Technology Centre.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
