diff options
author | Masatake YAMATO <yamato@redhat.com> | 2008-02-05 01:29:31 -0500 |
---|---|---|
committer | Linus Torvalds <torvalds@woody.linux-foundation.org> | 2008-02-05 12:44:19 -0500 |
commit | b5beb1caff4ce063e6e2dc13f23b80eeef4e9782 (patch) | |
tree | f3fcf8baafa1e9891edd7f16ffafd768c41c29d6 /mm | |
parent | e6f3602d2c58815775b2a293cec6650622236169 (diff) |
check ADVICE of fadvise64_64 even if get_xip_page is given
I've written some test programs in ltp project. During writing I met an
problem which I cannot solve in user land. So I wrote a patch for linux
kernel. Please, include this patch if acceptable.
The test program tests the 4th parameter of fadvise64_64:
long sys_fadvise64_64(int fd, loff_t offset, loff_t len, int advice);
My test case calls fadvise64_64 with invalid advice value and checks errno is
set to EINVAL. About the advice parameter man page says:
...
Permissible values for advice include:
POSIX_FADV_NORMAL
...
POSIX_FADV_SEQUENTIAL
...
POSIX_FADV_RANDOM
...
POSIX_FADV_NOREUSE
...
POSIX_FADV_WILLNEED
...
POSIX_FADV_DONTNEED
...
ERRORS
...
EINVAL An invalid value was specified for advice.
However, I got a bug report that the system call invocations
in my test case returned 0 unexpectedly.
I've inspected the kernel code:
asmlinkage long sys_fadvise64_64(int fd, loff_t offset, loff_t len, int advice)
{
struct file *file = fget(fd);
struct address_space *mapping;
struct backing_dev_info *bdi;
loff_t endbyte; /* inclusive */
pgoff_t start_index;
pgoff_t end_index;
unsigned long nrpages;
int ret = 0;
if (!file)
return -EBADF;
if (S_ISFIFO(file->f_path.dentry->d_inode->i_mode)) {
ret = -ESPIPE;
goto out;
}
mapping = file->f_mapping;
if (!mapping || len < 0) {
ret = -EINVAL;
goto out;
}
if (mapping->a_ops->get_xip_page)
/* no bad return value, but ignore advice */
goto out;
...
out:
fput(file);
return ret;
}
I found the advice parameter is just ignored in the case
mapping->a_ops->get_xip_page is given. This behavior is different from
what is written on the man page. Is this o.k.?
get_xip_page is given if CONFIG_EXT2_FS_XIP is true.
Anyway I cannot find the easy way to detect get_xip_page
field is given or CONFIG_EXT2_FS_XIP is true from the
user space.
I propose the following patch which checks the advice parameter
even if get_xip_page is given.
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Acked-by: Carsten Otte <cotte@de.ibm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'mm')
-rw-r--r-- | mm/fadvise.c | 16 |
1 files changed, 14 insertions, 2 deletions
diff --git a/mm/fadvise.c b/mm/fadvise.c index 0df4c899e979..3c0f1e99f5e4 100644 --- a/mm/fadvise.c +++ b/mm/fadvise.c | |||
@@ -49,9 +49,21 @@ asmlinkage long sys_fadvise64_64(int fd, loff_t offset, loff_t len, int advice) | |||
49 | goto out; | 49 | goto out; |
50 | } | 50 | } |
51 | 51 | ||
52 | if (mapping->a_ops->get_xip_page) | 52 | if (mapping->a_ops->get_xip_page) { |
53 | /* no bad return value, but ignore advice */ | 53 | switch (advice) { |
54 | case POSIX_FADV_NORMAL: | ||
55 | case POSIX_FADV_RANDOM: | ||
56 | case POSIX_FADV_SEQUENTIAL: | ||
57 | case POSIX_FADV_WILLNEED: | ||
58 | case POSIX_FADV_NOREUSE: | ||
59 | case POSIX_FADV_DONTNEED: | ||
60 | /* no bad return value, but ignore advice */ | ||
61 | break; | ||
62 | default: | ||
63 | ret = -EINVAL; | ||
64 | } | ||
54 | goto out; | 65 | goto out; |
66 | } | ||
55 | 67 | ||
56 | /* Careful about overflows. Len == 0 means "as much as possible" */ | 68 | /* Careful about overflows. Len == 0 means "as much as possible" */ |
57 | endbyte = offset + len; | 69 | endbyte = offset + len; |