Hi Linus,
When you fixed the mremap vulnerability, you said:
> I'd actually personally prefer a stronger test than the one in 2.4.24, and
> my personal preference would be for just disallowing the degenerate cases
> entirely. I don't see a "mremap away" as being a valid thing to do, since
> if that is what you want, why not just do a "munmap()"?
I think that your fix doesn't prevent these degenerate cases from
happening. To disallow them, the following patch should be applied:
--- linux-2.6.1-rc2/mm/mremap.c 2004-01-07 17:20:03.000000000 +0100
+++ linux/mm/mremap.c 2004-01-07 19:30:39.000000000 +0100
@@ -316,7 +316,7 @@
new_len = PAGE_ALIGN(new_len);
/* Don't allow the degenerate cases */
- if (!(old_len | new_len))
+ if (!old_len || !new_len)
goto out;
/* new_addr is only valid if MREMAP_FIXED is specified */
Here is a testing program which shows the difference:
/* mremap test by Michal Schmidt
* based on proof-of-concept exploit code for do_mremap()
* by Christophe Devine and Julien Tinnes
* GPL v2 */
#include <stdio.h>
#include <asm/unistd.h>
#include <sys/mman.h>
#include <unistd.h>
#include <errno.h>
#define MREMAP_MAYMOVE 1
#define MREMAP_FIXED 2
#define __NR_real_mremap __NR_mremap
static inline _syscall5( void *, real_mremap, void *, old_address,
size_t, old_size, size_t, new_size,
unsigned long, flags, void *, new_address );
void list_maps(void)
{
char str[50];
fflush(stdout);
sprintf(str,"cat /proc/%u/maps",getpid());
system(str);
}
int main( void )
{
void *base;
void *remapped;
printf("start\n");
list_maps();
base = mmap( NULL, 8192, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS, 0, 0 );
printf("mmap at: %p\n",base);
list_maps();
remapped=real_mremap( base, 8192, 0, MREMAP_MAYMOVE | MREMAP_FIXED,
(void *) 0xC0000000 );
printf("mremap to: %p\n",remapped);
list_maps();
return( 0 );
}
/* -----------EOF------------ */
Under 2.6.1-rc2 I get this output:
start
08048000-08049000 r-xp 00000000 03:05 355347 /home/michich/c/mremap
08049000-0804a000 rw-p 00000000 03:05 355347 /home/michich/c/mremap
40000000-40014000 r-xp 00000000 03:05 22062 /lib/ld-2.3.2.so
40014000-40015000 rw-p 00013000 03:05 22062 /lib/ld-2.3.2.so
40015000-40016000 rw-p 00000000 00:00 0
40026000-40152000 r-xp 00000000 03:05 22068 /lib/libc.so.6
40152000-40157000 rw-p 0012b000 03:05 22068 /lib/libc.so.6
40157000-40179000 rw-p 00000000 00:00 0
bfffe000-c0000000 rwxp fffff000 00:00 0
ffffe000-fffff000 ---p 00000000 00:00 0
mmap at: 0x40179000
08048000-08049000 r-xp 00000000 03:05 355347 /home/michich/c/mremap
08049000-0804a000 rw-p 00000000 03:05 355347 /home/michich/c/mremap
40000000-40014000 r-xp 00000000 03:05 22062 /lib/ld-2.3.2.so
40014000-40015000 rw-p 00013000 03:05 22062 /lib/ld-2.3.2.so
40015000-40016000 rw-p 00000000 00:00 0
40026000-40152000 r-xp 00000000 03:05 22068 /lib/libc.so.6
40152000-40157000 rw-p 0012b000 03:05 22068 /lib/libc.so.6
40157000-4017b000 rw-p 00000000 00:00 0
bfffe000-c0000000 rwxp fffff000 00:00 0
ffffe000-fffff000 ---p 00000000 00:00 0
mremap to: 0xffffffff
08048000-08049000 r-xp 00000000 03:05 355347 /home/michich/c/mremap
08049000-0804a000 rw-p 00000000 03:05 355347 /home/michich/c/mremap
40000000-40014000 r-xp 00000000 03:05 22062 /lib/ld-2.3.2.so
40014000-40015000 rw-p 00013000 03:05 22062 /lib/ld-2.3.2.so
40015000-40016000 rw-p 00000000 00:00 0
40026000-40152000 r-xp 00000000 03:05 22068 /lib/libc.so.6
40152000-40157000 rw-p 0012b000 03:05 22068 /lib/libc.so.6
40157000-40179000 rw-p 00000000 00:00 0
bfffe000-c0000000 rwxp fffff000 00:00 0
ffffe000-fffff000 ---p 00000000 00:00 0
.... so the mremap failed (it returned -1) but it has already unmapped
the area - it did the "mremap away" thing you wanted to prevent.
With my patch, mremap will also return -1, but doesn't change the memory
map - I believe that's better behaviour.
Michal Schmidt
On Wed, 7 Jan 2004, Michal Schmidt wrote:
>
> I think that your fix doesn't prevent these degenerate cases from
> happening.
Yeah, I'm an idiot. That's what you get for not actually testing your
patches. Thanks.
Linus
> That's what you get for not actually testing your
> patches. Thanks.
But I thought real men didn't test patches; they just
posted them to lkml and let others test them ... ;).
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.650.933.1373