2005-03-14 10:00:30

by Jin, Gordon

Subject: [PATCH 2.6] fix mprotect() with len=(size_t)(-1) to return -ENOMEM

This patch fixes a corner case in sys_mprotect():

Case: len is so large that will overflow to 0 after page alignment.
E.g. len=(size_t)(-1), i.e. 0xff...ff.
Expected result: POSIX spec says it should return -ENOMEM.
Current result: len is aligned to 0, then treated the same as len=0 and
return success.

--- linux-2.6.11.3/mm/mprotect.c.orig 2005-03-14 13:40:28.000000000
-0800
+++ linux-2.6.11.3/mm/mprotect.c 2005-03-14 13:42:41.000000000 -0800
@@ -232,14 +232,14 @@ sys_mprotect(unsigned long start, size_t

if (start & ~PAGE_MASK)
return -EINVAL;
+ if (!len)
+ return 0;
len = PAGE_ALIGN(len);
end = start + len;
- if (end < start)
+ if (end <= start)
return -ENOMEM;
if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM))
return -EINVAL;
- if (end == start)
- return 0;
/*
* Does the application expect PROT_READ to imply PROT_EXEC:
*/

2005-03-14 10:12:50

by Arjan van de Ven

Subject: Re: [PATCH 2.6] fix mprotect() with len=(size_t)(-1) to return -ENOMEM

On Mon, 2005-03-14 at 17:55 +0800, Gordon Jin wrote:
> This patch fixes a corner case in sys_mprotect():
>
> Case: len is so large that will overflow to 0 after page alignment.

shouldn't we just fix the alignment code instead that the overflow case
doesn't align to 0???
that sounds really odd.

2005-03-14 22:02:55

by Ingo Oeser

Subject: Re: [PATCH 2.6] fix mprotect() with len=(size_t)(-1) to return -ENOMEM

Hi Arjan,

You wrote:
> shouldn't we just fix the alignment code instead that the overflow case
> doesn't align to 0???
> that sounds really odd.

How? You have to align and you are out of bits for representing the
next number. What is the next number you can round to? "null" right!

Just remember that integer math with limited bits is always ring math ;-)

I love to abuse this for buffers and save an if.

Regards

Ingo Oeser

2005-03-15 03:09:06

by Jin, Gordon

Subject: [PATCH 2.6] fix mmap() return value to conform POSIX

This patch fixes 2 return values in mmap() to conform POSIX spec:

[EINVAL]
The value of len is zero.

[ENOMEM]
MAP_FIXED was specified, and the range [addr,addr+len) exceeds
that allowed for the address space of a process; or, if
MAP_FIXED was not specified and there is insufficient room in
the address space to effect the mapping.

--- linux-2.6.11.3/mm/mmap.c.orig 2005-03-14 13:20:11.000000000 -0800
+++ linux-2.6.11.3/mm/mmap.c 2005-03-14 17:24:37.000000000 -0800
@@ -897,12 +897,12 @@ unsigned long do_mmap_pgoff(struct file
prot |= PROT_EXEC;

if (!len)
- return addr;
+ return -EINVAL;

/* Careful about overflows.. */
len = PAGE_ALIGN(len);
if (!len || len > TASK_SIZE)
- return -EINVAL;
+ return -ENOMEM;

/* offset overflow? */
if ((pgoff + (len >> PAGE_SHIFT)) < pgoff)