Re: mmap() and threads BUG

Stephen C. Tweedie (sct@redhat.com)
Fri, 2 Apr 1999 17:05:15 +0100 (BST)

Hi,

On Thu, 1 Apr 1999 11:47:39 -0500 (EST), "Robert M. Fleischman"
<rmf@highwind.com> said:

> The following test program illustrates that when many threads do
> non-overlapping read's and write's to a shared file using mmap(), the
> data read does not match the data written.

Yes: reproduced here.

> Does this program work on the "2.2" kernels?

It runs just fine. 2.2 has extra locking around sys_brk(), sys_munmap()
and sys_mmap() to protect threaded applications.

> This program makes it appear that mmap() is really broken in 2.0.36.

It is. The patch below brings 2.0's mmap locking into line with 2.2's,
and should fix things: it certainly does here, at least on Intel. Could
you give it a try? I'll need to add the appropriate locking for other
architectures still: each platform has its own sys_mmap() front end, and
they will all need updating to match 2.2.

--Stephen

----------------------------------------------------------------
--- arch/i386/kernel/sys_i386.c~ Fri Apr 12 07:49:30 1996
+++ arch/i386/kernel/sys_i386.c Fri Apr 2 16:38:09 1999
@@ -48,19 +48,23 @@
int error;
unsigned long flags;
struct file * file = NULL;
+ unsigned long b[6];

error = verify_area(VERIFY_READ, buffer, 6*sizeof(long));
if (error)
return error;
- flags = get_user(buffer+3);
+ memcpy_fromfs(&b[0], buffer, 6*sizeof(long));
+ flags = b[3];
if (!(flags & MAP_ANONYMOUS)) {
- unsigned long fd = get_user(buffer+4);
+ unsigned long fd = b[4];
if (fd >= NR_OPEN || !(file = current->files->fd[fd]))
return -EBADF;
}
flags &= ~(MAP_EXECUTABLE | MAP_DENYWRITE);
- return do_mmap(file, get_user(buffer), get_user(buffer+1),
- get_user(buffer+2), flags, get_user(buffer+5));
+ down(&current->mm->mmap_sem);
+ error = do_mmap(file, b[0], b[1], b[2], flags, b[5]);
+ up(&current->mm->mmap_sem);
+ return error;
}

extern asmlinkage int sys_select(int, fd_set *, fd_set *, fd_set *, struct timeval *);
--- mm/mmap.c~ Fri Apr 2 14:43:11 1999
+++ mm/mmap.c Fri Apr 2 16:24:46 1999
@@ -69,20 +69,21 @@
unsigned long newbrk, oldbrk;
struct mm_struct *mm = current->mm;

+ down(&mm->mmap_sem);
+
if (brk < mm->end_code)
- return mm->brk;
+ goto out;
newbrk = PAGE_ALIGN(brk);
oldbrk = PAGE_ALIGN(mm->brk);
if (oldbrk == newbrk)
- return mm->brk = brk;
+ goto set_brk;

/*
* Always allow shrinking brk
*/
if (brk <= mm->brk) {
- mm->brk = brk;
do_munmap(newbrk, oldbrk-newbrk);
- return brk;
+ goto set_brk;
}
/*
* Check against rlimit and stack..
@@ -91,19 +92,19 @@
if (rlim >= RLIM_INFINITY)
rlim = ~0;
if (brk - mm->end_code > rlim)
- return mm->brk;
+ goto out;

/*
* Check against existing mmap mappings.
*/
if (find_vma_intersection(mm, oldbrk, newbrk+PAGE_SIZE))
- return mm->brk;
+ goto out;

/*
* Check if we have enough memory..
*/
if (!vm_enough_memory((newbrk-oldbrk) >> PAGE_SHIFT))
- return mm->brk;
+ goto out;

/*
* Ok, looks good - let it rip.
@@ -111,8 +112,12 @@
if(do_mmap(NULL, oldbrk, newbrk-oldbrk,
PROT_READ|PROT_WRITE|PROT_EXEC,
MAP_FIXED|MAP_PRIVATE, 0) != oldbrk)
- return mm->brk;
- return mm->brk = brk;
+ goto out;
+set_brk:
+ mm->brk = brk;
+out:
+ up(&mm->mmap_sem);
+ return mm->brk;
}

/*
@@ -784,7 +789,12 @@

asmlinkage int sys_munmap(unsigned long addr, size_t len)
{
- return do_munmap(addr, len);
+ int ret;
+
+ down(&current->mm->mmap_sem);
+ ret = do_munmap(addr, len);
+ up(&current->mm->mmap_sem);
+ return ret;
}

/*
@@ -796,7 +806,7 @@
int do_munmap(unsigned long addr, size_t len)
{
struct vm_area_struct *mpnt, *prev, *next, **npp, *free;
-
+
if ((addr & ~PAGE_MASK) || addr > MAX_USER_ADDR || len > MAX_USER_ADDR-addr)
return -EINVAL;

@@ -854,7 +864,6 @@
} while (free);

/* we could zap the page tables here too.. */
-
return 0;
}

@@ -980,10 +989,9 @@
{
struct vm_area_struct *prev, *mpnt, *next;

- down(&mm->mmap_sem);
mpnt = find_vma(mm, start_addr);
if (!mpnt)
- goto no_vma;
+ return;

avl_neighbours(mpnt, mm->mmap_avl, &prev, &next);
/* we have prev->vm_next == mpnt && mpnt->vm_next = next */
@@ -1043,6 +1051,4 @@
kfree_s(mpnt, sizeof(*mpnt));
mpnt = prev;
}
-no_vma:
- up(&mm->mmap_sem);
}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/