parisc: fix re-mmap() of shared file to same fixed address
Helge Deller
maintainerPATCH 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) = 0x42a54000mmap2(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.
arch/parisc/kernel/sys_parisc.c +15 -10
@@ -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; }
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)=0x42a54000mmap2(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_FIXEDandMAP_SHAREDflags 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
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)=0x42a54000mmap2(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_FIXEDandMAP_SHAREDflags 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=17db6e8d6b12f55e312fcab46faf5d332c806fb6it'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]
Helge Deller
maintainer-----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)=0x42a54000mmap2(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_FIXEDandMAP_SHAREDflags 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=17db6e8d6b12f55e312fcab46faf5d332c806fb6it'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-----