mincore returning -ENOMEM instead of -EFAULT

mincore returning -ENOMEM instead of -EFAULT

[Joel Becker]

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(&current->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: joel.becker@oracle.com
Phone: (650) 506-8127

[PATCH] mm/mincore: Return EFAULT when passed an invalid address.

[Joel Becker]

 	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 <joel.becker@oracle.com>

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: joel.becker@oracle.com
Phone: (650) 506-8127

Re: [PATCH] mm/mincore: Return EFAULT when passed an invalid address.

[Hugh Dickins]

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

Re: [PATCH] mm/mincore: Return EFAULT when passed an invalid address.

[Joel Becker]

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: joel.becker@oracle.com
Phone: (650) 506-8127

Re: mincore returning -ENOMEM instead of -EFAULT

[Linus Torvalds]

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