Linus,
Your fix in commit 2f77d107050abc14bc393b34bdb7b91cf670c250
modifies sys_mincore() to return -ENOMEM instead of -EFAULT on a totally
bogus address. Was this intentional, or is it something that should be
fixed up?
- /* check the output buffer whilst holding the lock */
- error = -EFAULT;
- down_read(¤t->mm->mmap_sem);
+ /* ..and we need to be passed a valid user-space range */
+ if (!access_ok(VERIFY_READ, (void __user *) start, len))
+ return -ENOMEM;
Joel
--
"Nobody loves me,
Nobody seems to care.
Troubles and worries, people,
You know I've had my share."
Joel Becker
Principal Software Developer
Oracle
E-mail: [email protected]
Phone: (650) 506-8127
The locking fix to sys_mincore in commit
2f77d107050abc14bc393b34bdb7b91cf670c250 returns -ENOMEM when given a
bad userspace address. It should return -EFAULT.
Signed-off-by: Joel Becker <[email protected]>
diff --git a/mm/mincore.c b/mm/mincore.c
index 8aca6f7..4af963c 100644
--- a/mm/mincore.c
+++ b/mm/mincore.c
@@ -122,7 +122,7 @@ asmlinkage long sys_mincore(unsigned long start, size_t len,
/* ..and we need to be passed a valid user-space range */
if (!access_ok(VERIFY_READ, (void __user *) start, len))
- return -ENOMEM;
+ return -EFAULT;
/* This also avoids any overflows on PAGE_CACHE_ALIGN */
pages = len >> PAGE_SHIFT;
--
"But then she looks me in the eye
And says, 'We're going to last forever,'
And man you know I can't begin to doubt it.
Cause it just feels so good and so free and so right,
I know we ain't never going to change our minds about it, Hey!
Here comes my girl."
Joel Becker
Principal Software Developer
Oracle
E-mail: [email protected]
Phone: (650) 506-8127
On Fri, 23 Feb 2007, Joel Becker wrote:
> The locking fix to sys_mincore in commit
> 2f77d107050abc14bc393b34bdb7b91cf670c250 returns -ENOMEM when given a
> bad userspace address. It should return -EFAULT.
No, I think you're getting confused by the way Linus uses access_ok
on the address range given with the access_ok on the vector address.
Before and after, an access_ok failure on the output vector address
gives -EFAULT. Before and after, an invalid address in the range
to be inspected gives -ENOMEM.
Which is consistent with the other m* system calls: -EFAULT if
arguments are inaccessible, -ENOMEM if address range is invalid.
Hugh
On Sat, Feb 24, 2007 at 01:05:27AM +0000, Hugh Dickins wrote:
> No, I think you're getting confused by the way Linus uses access_ok
> on the address range given with the access_ok on the vector address.
Whoops. Sorry for the noise.
Joel
--
Life's Little Instruction Book #451
"Don't be afraid to say, 'I'm sorry.'"
Joel Becker
Principal Software Developer
Oracle
E-mail: [email protected]
Phone: (650) 506-8127
On Fri, 23 Feb 2007, Joel Becker wrote:
>
> Your fix in commit 2f77d107050abc14bc393b34bdb7b91cf670c250
> modifies sys_mincore() to return -ENOMEM instead of -EFAULT on a totally
> bogus address. Was this intentional, or is it something that should be
> fixed up?
It was intentional, and I don't actually think it was even a change. The
access_ok() in question is not done on the actual array that we fill up
with the result of the operation, but on the address range that is being
queried, and if the queried address range is bogus, we should return
ENOMEM.
It's even mentioned in the man-page..
We *do* return -EFAULT for the case where the "vec" array itself is not
mapped.
Linus