LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* mincore returning -ENOMEM instead of -EFAULT
@ 2007-02-23 22:58 Joel Becker
  2007-02-23 23:11 ` [PATCH] mm/mincore: Return EFAULT when passed an invalid address Joel Becker
  2007-02-25 23:06 ` mincore returning -ENOMEM instead of -EFAULT Linus Torvalds
  0 siblings, 2 replies; 5+ messages in thread
From: Joel Becker @ 2007-02-23 22:58 UTC (permalink / raw)
  To: linux-kernel, Linus Torvalds

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] mm/mincore: Return EFAULT when passed an invalid address.
  2007-02-23 22:58 mincore returning -ENOMEM instead of -EFAULT Joel Becker
@ 2007-02-23 23:11 ` Joel Becker
  2007-02-24  1:05   ` Hugh Dickins
  2007-02-25 23:06 ` mincore returning -ENOMEM instead of -EFAULT Linus Torvalds
  1 sibling, 1 reply; 5+ messages in thread
From: Joel Becker @ 2007-02-23 23:11 UTC (permalink / raw)
  To: linux-kernel, Linus Torvalds

 	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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mm/mincore: Return EFAULT when passed an invalid address.
  2007-02-23 23:11 ` [PATCH] mm/mincore: Return EFAULT when passed an invalid address Joel Becker
@ 2007-02-24  1:05   ` Hugh Dickins
  2007-02-24  3:13     ` Joel Becker
  0 siblings, 1 reply; 5+ messages in thread
From: Hugh Dickins @ 2007-02-24  1:05 UTC (permalink / raw)
  To: Joel Becker; +Cc: linux-kernel, Linus Torvalds

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mm/mincore: Return EFAULT when passed an invalid address.
  2007-02-24  1:05   ` Hugh Dickins
@ 2007-02-24  3:13     ` Joel Becker
  0 siblings, 0 replies; 5+ messages in thread
From: Joel Becker @ 2007-02-24  3:13 UTC (permalink / raw)
  To: Hugh Dickins; +Cc: linux-kernel, Linus Torvalds

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: mincore returning -ENOMEM instead of -EFAULT
  2007-02-23 22:58 mincore returning -ENOMEM instead of -EFAULT Joel Becker
  2007-02-23 23:11 ` [PATCH] mm/mincore: Return EFAULT when passed an invalid address Joel Becker
@ 2007-02-25 23:06 ` Linus Torvalds
  1 sibling, 0 replies; 5+ messages in thread
From: Linus Torvalds @ 2007-02-25 23:06 UTC (permalink / raw)
  To: Joel Becker; +Cc: linux-kernel



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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2007-02-25 23:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-23 22:58 mincore returning -ENOMEM instead of -EFAULT Joel Becker
2007-02-23 23:11 ` [PATCH] mm/mincore: Return EFAULT when passed an invalid address Joel Becker
2007-02-24  1:05   ` Hugh Dickins
2007-02-24  3:13     ` Joel Becker
2007-02-25 23:06 ` mincore returning -ENOMEM instead of -EFAULT Linus Torvalds

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).