Skip to contentSkip to conversation

parisc: fix re-mmap() of shared file to same fixed address

PATCH In review Helge Deller 3 participants 4 messages Nov 20, 2013 – Nov 25, 2013 PA-RISC architecture

lore
Share & tools
lore

Helge Deller

author maintainer
#0 lore raw

PATCH 1/1 parisc: fix re-mmap() of shared file to same fixed address

locale-gen from glibc showed a strange problem on parisc:
mmap2(NULL, 536870912, PROT_NONE, MAP_SHARED, 3, 0) = 0x42a54000
mmap2(0x42a54000, 103860, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_FIXED, 3, 0) = -1 EINVAL (Invalid argument)

Basically it was just trying to re-mmap() a file at the same address which it was given by a previous mmap() call. But this remapping failed with EINVAL.

The problem is, that when MAP_FIXED and MAP_SHARED flags were used, we didn't included the mapping-based offset when we verified the alignment of the given fixed address against the offset which we calculated it in the previous call.

Cc stable....
arch/parisc/kernel/sys_parisc.c +15 -10
index 5dfd248..0d3a9d4 100644
@@ -61,8 +61,15 @@ static int get_offset(struct address_space *mapping)
 	return (unsigned long) mapping >> 8;
 }
 
-static unsigned long get_shared_area(struct address_space *mapping,
-		unsigned long addr, unsigned long len, unsigned long pgoff)
+static unsigned long shared_align_offset(struct file *filp, unsigned long pgoff)
+{
+	struct address_space *mapping = filp ? filp->f_mapping : NULL;
+
+	return (get_offset(mapping) + pgoff) << PAGE_SHIFT;
+}
+
+static unsigned long get_shared_area(struct file *filp, unsigned long addr,
+		unsigned long len, unsigned long pgoff)
 {
 	struct vm_unmapped_area_info info;
 
@@ -71,7 +78,7 @@ static unsigned long get_shared_area(struct address_space *mapping,
 	info.low_limit = PAGE_ALIGN(addr);
 	info.high_limit = TASK_SIZE;
 	info.align_mask = PAGE_MASK & (SHMLBA - 1);
-	info.align_offset = (get_offset(mapping) + pgoff) << PAGE_SHIFT;
+	info.align_offset = shared_align_offset(filp, pgoff);
 	return vm_unmapped_area(&info);
 }
 
@@ -82,20 +89,18 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
 		return -ENOMEM;
 	if (flags & MAP_FIXED) {
 		if ((flags & MAP_SHARED) &&
-		    (addr - (pgoff << PAGE_SHIFT)) & (SHMLBA - 1))
+		    (addr - shared_align_offset(filp, pgoff)) & (SHMLBA - 1))
 			return -EINVAL;
 		return addr;
 	}
 	if (!addr)
 		addr = TASK_UNMAPPED_BASE;
 
-	if (filp) {
-		addr = get_shared_area(filp->f_mapping, addr, len, pgoff);
-	} else if(flags & MAP_SHARED) {
-		addr = get_shared_area(NULL, addr, len, pgoff);
-	} else {
+	if (filp || (flags & MAP_SHARED))
+		addr = get_shared_area(filp, addr, len, pgoff);
+	else
 		addr = get_unshared_area(addr, len);
-	}
+
 	return addr;
 }
 
#1 lore raw
Quoted 1 line · On Wednesday 20 November 2013 17:07:42 Helge Deller wrote:

locale-gen from glibc showed a strange problem on parisc:

pedantic: locale-gen is from distros while glibc itself provides localedef.

Quoted 4 lines

mmap2(NULL, 536870912, PROT_NONE, MAP_SHARED, 3, 0) = 0x42a54000 mmap2(0x42a54000, 103860, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_FIXED, 3, 0) = -1 EINVAL (Invalid argument)

Quoted 8 lines

Basically it was just trying to re-mmap() a file at the same address which it was given by a previous mmap() call. But this remapping failed with EINVAL.

The problem is, that when MAP_FIXED and MAP_SHARED flags were used, we didn't included the mapping-based offset when we verified the alignment of the given fixed address against the offset which we calculated it in the previous call.

hmm, which version of glibc were you trying ? we landed this change recently:
https://sourceware.org/git/?p=glibc.git;a=commitdiff_plain;h=17db6e8d6b12f55e312fcab46faf5d332c806fb6

it's part of glibc-2.18. we know earlier versions of localedef failed on systems like parisc, although it seemed to really only creep up on 64bit kernels reliably.

based on your description, it sounds like independent bugs, but it also seems a little coincidental ...
-mike

#2 lore raw
Quoted 18 lines · Mike Frysinger
Quoted 1 line · On Wednesday 20 November 2013 17:07:42 Helge Deller wrote:

locale-gen from glibc showed a strange problem on parisc:

pedantic: locale-gen is from distros while glibc itself provides localedef.

Quoted 4 lines

mmap2(NULL, 536870912, PROT_NONE, MAP_SHARED, 3, 0) = 0x42a54000 mmap2(0x42a54000, 103860, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_FIXED, 3, 0) = -1 EINVAL (Invalid argument)

Quoted 8 lines

Basically it was just trying to re-mmap() a file at the same address which it was given by a previous mmap() call. But this remapping failed with EINVAL.

The problem is, that when MAP_FIXED and MAP_SHARED flags were used, we didn't included the mapping-based offset when we verified the alignment of the given fixed address against the offset which we calculated it in the previous call.

Quoted 6 lines

hmm, which version of glibc were you trying ? we landed this change recently:
https://sourceware.org/git/?p=glibc.git;a=commitdiff_plain;h=17db6e8d6b12f55e312fcab46faf5d332c806fb6

it's part of glibc-2.18. we know earlier versions of localedef failed on systems like parisc, although it seemed to really only creep up on 64bit kernels reliably.

Besides the above, our mmap allocater doesn't work well when we have a large number of small allocations:
https://rt.perl.org:443/rt3/Ticket/Display.html?id=119567

Effectively, we allocate 4 MB per locale and soon run out of memory.

Problem is here:

/*
* We need to know the offset to use. Old scheme was to look for
* existing mapping and use the same offset. New scheme is to use the
* address of the kernel data structure as the seed for the offset.
* We'll see how that works...
*
* The mapping is cacheline aligned, so there's no information in the
bottom
* few bits of the address. We're looking for 10 bits (4MB / 4k), so
let's
* drop the bottom 8 bits and use bits 8-17.
*/
static int get_offset(struct address_space *mapping)

{
        return (unsigned long) mapping >> 8;
}

We rarely suceed in allocating two or more maps in the same 4 MB block of memory.

Dave
--
John David Anglin [email protected]

author maintainer
#3 lore raw
In reply to Mike Frysinger

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Mike,

Quoted 4 lines · Mike Frysinger
Quoted 1 line · On Wednesday 20 November 2013 17:07:42 Helge Deller wrote:

locale-gen from glibc showed a strange problem on parisc:

pedantic: locale-gen is from distros while glibc itself provides localedef.

Ok.

Quoted 13 lines
Quoted 4 lines

mmap2(NULL, 536870912, PROT_NONE, MAP_SHARED, 3, 0) = 0x42a54000 mmap2(0x42a54000, 103860, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_FIXED, 3, 0) = -1 EINVAL (Invalid argument)

Quoted 8 lines

Basically it was just trying to re-mmap() a file at the same address which it was given by a previous mmap() call. But this remapping failed with EINVAL.

The problem is, that when MAP_FIXED and MAP_SHARED flags were used, we didn't included the mapping-based offset when we verified the alignment of the given fixed address against the offset which we calculated it in the previous call.

Quoted 6 lines

hmm, which version of glibc were you trying ? we landed this change recently:
https://sourceware.org/git/?p=glibc.git;a=commitdiff_plain;h=17db6e8d6b12f55e312fcab46faf5d332c806fb6

it's part of glibc-2.18. we know earlier versions of localedef failed on systems like parisc, although it seemed to really only creep up on 64bit kernels reliably.

It's debian's glibc 2.17-96

Quoted 2 lines

based on your description, it sounds like independent bugs, but it also seems a little coincidental ...

It's at least related.
As long as in the second mmap(ptr,...) the ptr value given is the same as the ptr value returned by a prior call to mmap(NULL,...) it works.
It's hard to read from the diff if this is the case with the new glibc patch...

Anyway, I think the main thing one should know is, that a mmap(NULL,..) call will in most cases *not* return a value which is a multiple of SHMLBA. But the offset (ptr%SHMLBA) will stay constant as long as the same file handle is used.

Helge
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJSk7Z5AAoJEKGDlV8wpRJBPeQP/RndkCBrSEwNbH5HWFS332Lw CJ/l5QkG2rhZh1EYKywoxqlYeTR3vDveacCQEuOozeVm7zI50leZACFI8XromoTb 3MQ0oPy3X+ApP+X/+KYSO/5ew+bRMcJJ3bmid9jdZ8kA3U4h6nsdE1dd3tJl1sf0 10Jm/0OPz+x0mJBZU8V4cuZPllHsyvfk7IN/vnfhy3HdW/Jq2IPPE+XZhri+3SQu qBSnZOVts6MS3tlC757GgKhGGLjer4hVrJaC4BQucSoAk3fEJZqXQzcs9HDIWTNd k4kkan6fnUdvxpvZOO//TYeAYDZQvgMiYju/CMw35B+Iyk9Anma3oXxtTQTbaFh5 zjPlsrCuU+V7oUag6WiLKKtlmUg07sdX+G4Htf64mT9avdQvuGwWlJieORg7Xnjw /32tWjyw/j/v6+Omdq5tMf+Si7NeDi+AbE2IaRFrGL+/1ANXj2XoTRP2o34gMj90 7OL6/ntV/p+D/ByGmSlGbvSL7RzKnYxHNS1/reeEys+tS2j3+C0M+7GDm1/r3BUV hXVh0eFizr1p2tDsabCEbil3lEffVPXJ12mVY+HDhe7KGav66suNvhaqfcou07Yr j9arKCVZyjbXkOmTjRXrGpgs697yqail100U3VuNXgrdBJz98oHbSB4t1jG5K/CW TePZPuUjmHyjWTwyFRPp
=IeWC
-----END PGP SIGNATURE-----