This patch fixes the bug identified during testing of golang support in OSv. 
More 
specifically invocations of SYSCALL instruction involving 6 arguments (plus 
syscall number)
like pselect6 or mmap would often fail with last 6th argument carrying wrong 
random value
diffent from what caller would pass in. 

As Nadav explains in OSv forum the original syscall_wrapper implementation 
assumed 
that the arguments received would be passed correctly on the stack to syscall 
function
as is which was NOT the case and led to the corruption of the 6th argument. So 
instead
new syscall_wrapper implementation explicitly calls syscall() function with all
seven parameters explicitly passed in. 

The fix is authored by Nadav Har'El and new unit test in tst-syscall.cc added 
by Waldemar Kozaczuk.

Signed-off-by: Waldemar Kozaczuk <[email protected]>
---
 linux.cc             | 11 ++++++++---
 tests/tst-syscall.cc | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/linux.cc b/linux.cc
index a0ceb50..d8ed544 100644
--- a/linux.cc
+++ b/linux.cc
@@ -352,15 +352,20 @@ long syscall(long number, ...)
 }
 long __syscall(long number, ...)  __attribute__((alias("syscall")));
 
-extern "C" long syscall_wrapper(long number, ...)
+// In x86-64, a SYSCALL instruction has exactly 6 parameters, because this is 
the number of registers
+// alloted for passing them (additional parameters *cannot* be passed on the 
stack). So we can get
+// 7 arguments to this function (syscall number plus its 6 parameters). 
Because in the x86-64 ABI the
+// seventh argument is on the stack, we must pass the arguments explicitly to 
the syscall() function
+// and can't just call it without any arguments and hope everything will be 
passed on
+extern "C" long syscall_wrapper(long number, long p1, long p2, long p3, long 
p4, long p5, long p6)
 {
     int errno_backup = errno;
     // syscall and function return value are in rax
-    auto ret = syscall(number);
+    auto ret = syscall(number, p1, p2, p3, p4, p5, p6);
     int result = -errno;
     errno = errno_backup;
     if (ret < 0 && ret >= -4096) {
-       return result;
+        return result;
     }
     return ret;
 }
diff --git a/tests/tst-syscall.cc b/tests/tst-syscall.cc
index e7278cc..5e7ed4a 100644
--- a/tests/tst-syscall.cc
+++ b/tests/tst-syscall.cc
@@ -8,6 +8,11 @@
 #include <syscall.h>
 #include <unistd.h>
 #include <errno.h>
+#include <sys/mman.h>
+#include <cassert>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
 
 #include <iostream>
 
@@ -52,6 +57,34 @@ int main(int argc, char **argv)
     expect(tid >= 0, true);
     expect(tid, gettid());
 
+    // test mmap as it takes 6 parameters
+    int fd = open("/tests/tst-mmap.so", O_RDONLY, 0666);
+    assert(fd > 0);
+
+    void *addr = NULL;
+    size_t length = 8192;
+    int prot = PROT_READ;
+    int flags = MAP_PRIVATE;
+    off_t offset = 0;
+    void* buf = NULL;
+
+    asm ("movq %[addr], %%rdi\n"
+         "movq %[length], %%rsi\n"
+         "movl %[prot], %%edx\n"
+         "movq %[flags], %%r10\n"
+         "movq %[fd], %%r8\n"
+         "movq %[offset], %%r9\n"
+         "movq $9, %%rax\n"
+         "syscall\n"
+         "movq %%rax, %[buf]\n"
+         : [buf] "=m" (buf)
+         : [addr] "m" (addr), [length] "m" (length), [prot] "m" (prot), 
[flags] "m" (flags), [fd] "m" (fd), [offset] "m" (offset));
+
+    assert(((long)buf) >= 0);
+    munmap(buf, length);
+
+    assert(close(fd) == 0);
+
     // test that unknown system call results in a ENOSYS (see issue #757)
     expect_errno_l(syscall(999), ENOSYS);
 
-- 
2.7.4

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to