From fe203ffbb2bd7f93a86013d341aa767a406150bc Mon Sep 17 00:00:00 2001 From: mikulas-patocka <94898783+mikulas-patocka@users.noreply.github.com> Date: Thu, 27 Mar 2025 01:31:49 +0100 Subject: [PATCH] Fix bugs in the x86-64 and x32 target (#887) (#889) This commit fixes two bugs in ffi in the x86-64 target. The bugs were introduced by the commit d21881f55ed4a44d464c9091871e69b0bb47611a ("Fix x86/ffi64 calls with 6 gp and some sse registers"). The first bug is that when we pass an argument with less than 8 bytes, ffi will read memory beyond argument end, causing a crash if the argument is located just before the end of the mapped region. The second bug is in the x32 ABI - pointers in x32 are 4-byte, but GCC assumes that the pointer values in the registers are zero-extended. ffi doesn't respect this assumption, causing crashes in the called library. For example, when we compile this function for x32: int fn(int *a) { if (a) return *a; return -1; } we get this code: fn: testq %rdi, %rdi je .L3 movl (%edi), %eax ret .L3: movl $-1, %eax ret When we call this function using ffi with the argument NULL, the function crashes because top 4 bytes of the RDI register are not cleared. Fixes: d21881f55ed4 ("Fix x86/ffi64 calls with 6 gp and some sse registers (#848)") Signed-off-by: Mikulas Patocka --- src/x86/ffi64.c | 2 +- testsuite/libffi.call/overread.c | 54 ++++++++++++++++++++++++++++++++ testsuite/libffi.call/x32.c | 31 ++++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 testsuite/libffi.call/overread.c create mode 100644 testsuite/libffi.call/x32.c diff --git a/src/x86/ffi64.c b/src/x86/ffi64.c index 982b144c..1d1b88e0 100644 --- a/src/x86/ffi64.c +++ b/src/x86/ffi64.c @@ -654,7 +654,7 @@ ffi_call_int (ffi_cif *cif, void (*fn)(void), void *rvalue, break; default: reg_args->gpr[gprcount] = 0; - memcpy (®_args->gpr[gprcount], a, sizeof(UINT64)); + memcpy (®_args->gpr[gprcount], a, size <= 8 ? size : 8); } gprcount++; break; diff --git a/testsuite/libffi.call/overread.c b/testsuite/libffi.call/overread.c new file mode 100644 index 00000000..d9ae579b --- /dev/null +++ b/testsuite/libffi.call/overread.c @@ -0,0 +1,54 @@ +/* Area: ffi_call + Purpose: Tests if ffi_call reads data beyond end. + Limitations: needs mmap. + PR: 887 + Originator: Mikulas Patocka */ + +/* { dg-do run } */ + +#include "ffitest.h" + +#ifdef __linux__ +#include +#include + +static int fn(unsigned char a, unsigned short b, unsigned int c, unsigned long d) +{ + return (int)(a + b + c + d); +} +#endif + +int main(void) +{ +#ifdef __linux__ + ffi_cif cif; + ffi_type *args[MAX_ARGS]; + void *values[MAX_ARGS]; + ffi_arg rint; + char *m; + int ps; + args[0] = &ffi_type_uchar; + args[1] = &ffi_type_ushort; + args[2] = &ffi_type_uint; + args[3] = &ffi_type_ulong; + CHECK(ffi_prep_cif(&cif, ABI_NUM, 4, &ffi_type_sint, args) == FFI_OK); + ps = getpagesize(); + m = mmap(NULL, ps * 3, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); + CHECK(m != MAP_FAILED); + CHECK(mprotect(m, ps, PROT_NONE) == 0); + CHECK(mprotect(m + ps * 2, ps, PROT_NONE) == 0); + values[0] = m + ps * 2 - sizeof(unsigned char); + values[1] = m + ps * 2 - sizeof(unsigned short); + values[2] = m + ps * 2 - sizeof(unsigned int); + values[3] = m + ps * 2 - sizeof(unsigned long); + ffi_call(&cif, FFI_FN(fn), &rint, values); + CHECK((int)rint == 0); + values[0] = m + ps; + values[1] = m + ps; + values[2] = m + ps; + values[3] = m + ps; + ffi_call(&cif, FFI_FN(fn), &rint, values); + CHECK((int)rint == 0); +#endif + exit(0); +} diff --git a/testsuite/libffi.call/x32.c b/testsuite/libffi.call/x32.c new file mode 100644 index 00000000..f9ad82de --- /dev/null +++ b/testsuite/libffi.call/x32.c @@ -0,0 +1,31 @@ +/* Area: ffi_call + Purpose: Check zero-extension of pointers on x32. + Limitations: none. + PR: 887 + Originator: Mikulas Patocka */ + +/* { dg-do run } */ + +#include "ffitest.h" + +static int fn(int *a) +{ + if (a) + return *a; + return -1; +} + +int main(void) +{ + ffi_cif cif; + ffi_type *args[MAX_ARGS]; + void *values[MAX_ARGS]; + void *z[2] = { (void *)0, (void *)1 }; + ffi_arg rint; + args[0] = &ffi_type_pointer; + values[0] = z; + CHECK(ffi_prep_cif(&cif, ABI_NUM, 1, &ffi_type_sint, args) == FFI_OK); + ffi_call(&cif, FFI_FN(fn), &rint, values); + CHECK((int)rint == -1); + exit(0); +}