Message ID | 55DF7F32.2010308@linaro.org |
---|---|
State | New |
Headers | show |
On Thu, Aug 27, 2015 at 06:20:50PM -0300, Adhemerval Zanella wrote: [...] > Thanks for the review, what about now: > > -- > > [BZ #18877] > * posix/Makefile (tests): Add tst-mmap It's called tst-mmap-offset now. Do not forget about trailing dot. > * sysdeps/unix/sysv/linux/generic/wordsize-32/mmap.c (__mmap): Fix > offset calculation for negative values. > * posix/tst-mmap-offset.c: New file. Let's keep the list of files in some order. > --- /dev/null > +++ b/posix/tst-mmap-offset.c > @@ -0,0 +1,67 @@ > +/* BZ #18877 mmap offset test. > + > + Copyright (C) 2015 Free Software Foundation, Inc. > + This file is part of the GNU C Library. Contributed by me, 2015. :) > +/* Check if negative offset are handled correctly by mmap. */; offsetS are. The trailing semicolon is not needed.
On 27-08-2015 18:48, Dmitry V. Levin wrote: > On Thu, Aug 27, 2015 at 06:20:50PM -0300, Adhemerval Zanella wrote: > [...] >> Thanks for the review, what about now: >> >> -- >> >> [BZ #18877] >> * posix/Makefile (tests): Add tst-mmap > > It's called tst-mmap-offset now. > Do not forget about trailing dot. > >> * sysdeps/unix/sysv/linux/generic/wordsize-32/mmap.c (__mmap): Fix >> offset calculation for negative values. >> * posix/tst-mmap-offset.c: New file. > > Let's keep the list of files in some order. > >> --- /dev/null >> +++ b/posix/tst-mmap-offset.c >> @@ -0,0 +1,67 @@ >> +/* BZ #18877 mmap offset test. >> + >> + Copyright (C) 2015 Free Software Foundation, Inc. >> + This file is part of the GNU C Library. > > Contributed by me, 2015. :) We do not use 'Contributed by' anymore, we use it the ChangeLog entry (which I will add you as well). > >> +/* Check if negative offset are handled correctly by mmap. */; > > offsetS are. > The trailing semicolon is not needed. > >
On Thu, Aug 27, 2015 at 08:06:55PM -0300, Adhemerval Zanella wrote: > On 27-08-2015 18:48, Dmitry V. Levin wrote: > > On Thu, Aug 27, 2015 at 06:20:50PM -0300, Adhemerval Zanella wrote: > > [...] > >> Thanks for the review, what about now: > >> > >> -- > >> > >> [BZ #18877] > >> * posix/Makefile (tests): Add tst-mmap > > > > It's called tst-mmap-offset now. > > Do not forget about trailing dot. > > > >> * sysdeps/unix/sysv/linux/generic/wordsize-32/mmap.c (__mmap): Fix > >> offset calculation for negative values. > >> * posix/tst-mmap-offset.c: New file. > > > > Let's keep the list of files in some order. > > > >> --- /dev/null > >> +++ b/posix/tst-mmap-offset.c > >> @@ -0,0 +1,67 @@ > >> +/* BZ #18877 mmap offset test. > >> + > >> + Copyright (C) 2015 Free Software Foundation, Inc. > >> + This file is part of the GNU C Library. > > > > Contributed by me, 2015. :) > > We do not use 'Contributed by' anymore, we use it the ChangeLog entry > (which I will add you as well). Indeed. > >> +/* Check if negative offset are handled correctly by mmap. */; > > > > offsetS are. > > The trailing semicolon is not needed. LGTM, assuming these typos are corrected.
On 28-08-2015 04:05, Dmitry V. Levin wrote: > On Thu, Aug 27, 2015 at 08:06:55PM -0300, Adhemerval Zanella wrote: >> On 27-08-2015 18:48, Dmitry V. Levin wrote: >>> On Thu, Aug 27, 2015 at 06:20:50PM -0300, Adhemerval Zanella wrote: >>> [...] >>>> Thanks for the review, what about now: >>>> >>>> -- >>>> >>>> [BZ #18877] >>>> * posix/Makefile (tests): Add tst-mmap >>> >>> It's called tst-mmap-offset now. >>> Do not forget about trailing dot. >>> >>>> * sysdeps/unix/sysv/linux/generic/wordsize-32/mmap.c (__mmap): Fix >>>> offset calculation for negative values. >>>> * posix/tst-mmap-offset.c: New file. >>> >>> Let's keep the list of files in some order. >>> >>>> --- /dev/null >>>> +++ b/posix/tst-mmap-offset.c >>>> @@ -0,0 +1,67 @@ >>>> +/* BZ #18877 mmap offset test. >>>> + >>>> + Copyright (C) 2015 Free Software Foundation, Inc. >>>> + This file is part of the GNU C Library. >>> >>> Contributed by me, 2015. :) >> >> We do not use 'Contributed by' anymore, we use it the ChangeLog entry >> (which I will add you as well). > > Indeed. > >>>> +/* Check if negative offset are handled correctly by mmap. */; >>> >>> offsetS are. >>> The trailing semicolon is not needed. > > LGTM, assuming these typos are corrected. > Pushed upstream as d3573f61aca67a398de7eaa7593d3973cb5fd154.
diff --git a/posix/Makefile b/posix/Makefile index 15e8818..39423a9 100644 --- a/posix/Makefile +++ b/posix/Makefile @@ -64,7 +64,7 @@ routines := \ aux := init-posix environ tests := tstgetopt testfnm runtests runptests \ tst-preadwrite tst-preadwrite64 test-vfork regexbug1 \ - tst-mmap tst-getaddrinfo tst-truncate \ + tst-mmap tst-mmap-offset tst-getaddrinfo tst-truncate \ tst-truncate64 tst-fork tst-fnmatch tst-regexloc tst-dir \ tst-chmod bug-regex1 bug-regex2 bug-regex3 bug-regex4 \ tst-gnuglob tst-regex bug-regex5 bug-regex6 bug-regex7 \ diff --git a/posix/tst-mmap-offset.c b/posix/tst-mmap-offset.c new file mode 100644 index 0000000..15dc4da --- /dev/null +++ b/posix/tst-mmap-offset.c @@ -0,0 +1,67 @@ +/* BZ #18877 mmap offset test. + + Copyright (C) 2015 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <sys/mman.h> + +static int +printmsg (int rc, const char *msg) +{ + printf ("%s failed: %m\n", msg); + return rc; +} + +/* Check if negative offset are handled correctly by mmap. */; +static int +do_test (void) +{ + const int prot = PROT_READ | PROT_WRITE; + const int flags = MAP_SHARED; + const unsigned long length = 0x10000; + const unsigned long offset = 0xace00000; + const unsigned long size = offset + length; + void *addr; + int fd; + char fname[] = "tst-mmap-offset-XXXXXX"; + + fd = mkstemp64 (fname); + if (fd < 0) + return printmsg (1, "mkstemp"); + + if (unlink (fname)) + return printmsg (1, "unlink"); + + if (ftruncate64 (fd, size)) + return printmsg (0, "ftruncate64"); + + addr = mmap (NULL, length, prot, flags, fd, offset); + if (MAP_FAILED == addr) + return printmsg (1, "mmap"); + + /* This memcpy is likely to SIGBUS if mmap has messed up with offset. */ + memcpy (addr, fname, sizeof (fname)); + + return 0; +} + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/mmap.c b/sysdeps/unix/sysv/linux/generic/wordsize-32/mmap.c index 24835ce..82d8920 100644 --- a/sysdeps/unix/sysv/linux/generic/wordsize-32/mmap.c +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/mmap.c @@ -21,20 +21,20 @@ #include <errno.h> #include <sysdep.h> -#ifndef MMAP_PAGE_SHIFT -#define MMAP_PAGE_SHIFT 12 +#ifndef MMAP_PAGE_UNIT +#define MMAP_PAGE_UNIT 4096UL #endif __ptr_t __mmap (__ptr_t addr, size_t len, int prot, int flags, int fd, off_t offset) { - if (offset & ((1 << MMAP_PAGE_SHIFT) - 1)) + if (offset & (MMAP_PAGE_UNIT - 1)) { __set_errno (EINVAL); return MAP_FAILED; } return (__ptr_t) INLINE_SYSCALL (mmap2, 6, addr, len, prot, flags, fd, - offset >> MMAP_PAGE_SHIFT); + offset / MMAP_PAGE_UNIT); } weak_alias (__mmap, mmap)