mmap: fix petty bug in anonymous shared mmap offset handling

PATCH In review Tejun Heo 3 participants 3 messages Sep 3, 2008 – Sep 4, 2008

Subsystems (2)

Memory management Memory mapping

lore

Share & tools

lore

Tejun Heo

author

2008-09-03 14:09 UTC #0 lore raw

PATCH 1/1 mmap: fix petty bug in anonymous shared mmap offset handling

Anonymous mappings should ignore offset but shared anonymous mapping forgot to clear it and makes the following legit test program trigger SIGBUS.

#include <sys/mman.h>
#include <stdio.h>
#include <errno.h>

#define PAGE_SIZE 4096

int main(void)

 {
	 char *p;
	 int i;

	 p = mmap(NULL, 2 * PAGE_SIZE, PROT_READ|PROT_WRITE,
		  MAP_SHARED|MAP_ANONYMOUS, -1, PAGE_SIZE);
	 if (p == MAP_FAILED) {
		 perror("mmap");
		 return 1;
	 }

	 for (i = 0; i < 2; i++) {
		 printf("page %d\n", i);
		 p[i * 4096] = i;
	 }
	 return 0;

}

Fix it.

Signed-off-by Tejun Heo <[email protected]>

---
 mm/mmap.c |    4 ++++
 1 file changed, 4 insertions(+)

mm/mmap.c +4 -0

index 339cf5c..e7a5a68 100644
@@ -1030,6 +1030,10 @@ unsigned long do_mmap_pgoff(struct file * file, unsigned long addr,
 	} else {
 		switch (flags & MAP_TYPE) {
 		case MAP_SHARED:
+			/*
+			 * Ignore pgoff.
+			 */
+			pgoff = 0;
 			vm_flags |= VM_SHARED | VM_MAYSHARE;
 			break;
 		case MAP_PRIVATE:

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Hugh Dickins

Acked-by on v1 1/1

2008-09-03 16:25 UTC #1 lore raw

Quoted 27 lines · Tejun Heo

Anonymous mappings should ignore offset but shared anonymous mapping forgot to clear it and makes the following legit test program trigger SIGBUS.
#include <sys/mman.h>
#include <stdio.h>
#include <errno.h>
#define PAGE_SIZE 4096
int main(void)
 {
	 char *p;
	 int i;

	 p = mmap(NULL, 2 * PAGE_SIZE, PROT_READ|PROT_WRITE,
		  MAP_SHARED|MAP_ANONYMOUS, -1, PAGE_SIZE);
	 if (p == MAP_FAILED) {
		 perror("mmap");
		 return 1;
	 }

	 for (i = 0; i < 2; i++) {
		 printf("page %d\n", i);
		 p[i * 4096] = i;
	 }
	 return 0;

Quoted 5 lines

}
Fix it.
Signed-off-by Tejun Heo <[email protected]>

Fair enough. The current behaviour has (almost) never bothered us, so I'm uncertain if your test is legit, but I can't see any reason to object to the change. Particularly since (just out of sight below the context of your patch) we force pgoff in the MAP_PRIVATE case.

Acked-by Hugh Dickins <[email protected]>

Quoted 9 lines

---
 mm/mmap.c |    4 ++++
 1 file changed, 4 insertions(+)

mm/mmap.c +0 -0

index 339cf5c..e7a5a68 100644

mm/mmap.c unsigned long do_mmap_pgoff(struct file * file, unsigned long addr, · line 1039

Quoted 10 lines

mm/mmap.c +4 -0

@@ -1030,6 +1030,10 @@ unsigned long do_mmap_pgoff(struct file * file, unsigned long addr,
 	} else {
 		switch (flags & MAP_TYPE) {
 		case MAP_SHARED:
+			/*
+			 * Ignore pgoff.
+			 */
+			pgoff = 0;
 			vm_flags |= VM_SHARED | VM_MAYSHARE;
 			break;
 		case MAP_PRIVATE:

KOSAKI Motohiro

Acked-by

2008-09-04 02:51 UTC #2 lore raw

Quoted 8 lines

Quoted 1 line
Signed-off-by Tejun Heo <[email protected]>
Fair enough. The current behaviour has (almost) never bothered us, so I'm uncertain if your test is legit, but I can't see any reason to object to the change. Particularly since (just out of sight below the context of your patch) we force pgoff in the MAP_PRIVATE case.
Acked-by Hugh Dickins <[email protected]>

me too.

Acked-by KOSAKI Motohiro <[email protected]>