Review Board 2.0.15


Invalid alignment checks in mmap and mremap

Review Request #1735 - Created Feb. 22, 2013 and submitted

Information
Tom Jablin
gem5
Reviewers
Default
Presently, the alignment checks in the mmap and mremap implementations in syscall_emul.hh are wrong. The checks are implemented as:

if ((start % TheISA::VMPageSize) != 0 ||
(length % TheISA::VMPageSize) != 0) {
warn("mmap failing: arguments not page-aligned: "
"start 0x%x length 0x%x",
start, length);
return -EINVAL;
}

This checks that both the start and the length arguments of the mmap syscall are checked for page-alignment. However, the POSIX specification says:

The off argument is constrained to be aligned and sized according to the value returned by sysconf() when passed _SC_PAGESIZE or _SC_PAGE_SIZE. When MAP_FIXED is specified, the application shall ensure that the argument addr also meets these constraints. The implementation performs mapping operations over whole pages. Thus, while the argument len need not meet a size or alignment constraint, the implementation shall include, in any mapping operation, any partial page specified by the range [pa,pa+len).

So the length parameter should not be checked for page-alignment. By contrast, the current implementation fails to check the offset argument, which must be page aligned.

I do not have commit privileges, so can a reviewer please push this patch for me? 
#include <stdio.h>
#include <unistd.h>
#include <sys/mman.h>

int main(int argc, char **argv) {

  void *addr1 =
    mmap(0, 1, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
  printf("Test unaligned lengths: %s\n",
         addr1 == (void *) -1 ? "Failed" : "Passed");

  size_t pageSize = (size_t) sysconf(_SC_PAGE_SIZE);
  void *addr2 =
    mmap(0, pageSize, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 1);
  printf("Test unaligned offsets: %s\n",
         addr2 == (void *) -1 ? "Passed" : "Failed");

  return 0;
}

Issue Summary

2 2 0 0
Description From Last Updated Status
This should be new_length = roundUp(new_length, TheISA::VMPageSize); Ali Saidi July 15, 2013, 8:35 a.m. Open
This should be length = roundUp(length, TheISA::VMPageSize); Ali Saidi July 15, 2013, 8:35 a.m. Open
Review request changed
Updated (Oct. 20, 2014, 2:42 p.m.)

Status: Closed (submitted)