Bug 360574 - Wrong parameter type for an ashmem ioctl() call on Android and ARM64
Summary: Wrong parameter type for an ashmem ioctl() call on Android and ARM64
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (show other bugs)
Version: 3.11.0
Platform: Android Unspecified
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-15 18:50 UTC by Anton Kirilov
Modified: 2016-08-04 21:49 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Anton Kirilov 2016-03-15 18:50:00 UTC
The ASHMEM_SET_SIZE ioctl() call accepts a size as a parameter, not a pointer, which is not recognized by Valgrind on ARM64, and results in errors such as:

==2040== Syscall param ioctl(generic) points to unaddressable byte(s)
==2040==    at 0x496F768: __ioctl (in /system/lib64/libc.so)
==2040==    by 0x492530B: ioctl (in /system/lib64/libc.so)
==2040==    by 0x5A9B04B: ashmem_create_region (in /system/lib64/libcutils.so)
==2040==    by 0x5768BA3: art::MemMap::MapAnonymous(char const*, unsigned char*, unsigned long, int, bool, bool, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*, bool) (in /system/lib64/libart.so)
==2040==    by 0x5612247: art::gc::Heap::Heap(unsigned long, unsigned long, unsigned long, unsigned long, double, double, unsigned long, unsigned long, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, art::InstructionSet, art::gc::CollectorType, art::gc::CollectorType, art::gc::space::LargeObjectSpaceType, unsigned long, unsigned long, unsigned long, bool, unsigned long, unsigned long, bool, bool, bool, bool, bool, bool, bool, bool, bool, bool, unsigned long) (in /system/lib64/libart.so)
==2040==    by 0x583BCFB: art::Runtime::Init(art::RuntimeArgumentMap&&) (in /system/lib64/libart.so)
==2040==    by 0x583E823: art::Runtime::Create(std::__1::vector<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, void const*>, std::__1::allocator<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, void const*> > > const&, bool) (in /system/lib64/libart.so)
==2040==    by 0x5705093: JNI_CreateJavaVM (in /system/lib64/libart.so)
==2040==    by 0x109377: main (in /system/bin/dalvikvm64)
==2040==  Address 0x10000000 is not stack'd, malloc'd or (recently) free'd

I have seen the issue with a build of the master branch of the Android Open Source Project, but it is reproducible on older versions as well (e.g. the 6.0 release).

Reproducible: Always

Steps to Reproduce:
1. Install the ARM64 version of Valgrind on the Android device as documented in the README.android file; assume that the installation directory is /data/local/tmp/valgrind.
2. Get an APK or a DEX file; let its name be Test.dex, and let the name of the class containing the main() method be Test as well.
3. adb push Test.dex /data/local/tmp
4. adb shell /data/local/tmp/valgrind/bin/valgrind dalvikvm64 -cp /data/local/tmp/Test.dex Test

Actual Results:  
Errors similar to the one above are displayed.

Expected Results:  
No errors of that type are shown.

Note that Valgrind is not affected by the same issue on a 32-bit ARM platform.

I have been using a suppression rule as a workaround:

{
  ashmem ioctl
  Memcheck:Param
  ioctl(generic)
  ...
  fun:ioctl
  fun:ashmem_create_region
}
Comment 1 Julian Seward 2016-08-03 12:26:40 UTC
Anton: I don't have an easily available arm64-android device to test on.  But I 
think this is an easy fix.  If I get you a patch, can you test it for me?
Comment 2 Anton Kirilov 2016-08-03 18:06:47 UTC
Yes, I can.
Comment 3 Julian Seward 2016-08-04 06:32:21 UTC
The problem is that the wrappers for android-specific ioctls on 64-bit ARM
are not enabled, so it handles them using the generic ioctl logic, which
in this case isn't appropriate.  Can you try this patch?


Index: coregrind/m_syswrap/syswrap-linux.c
===================================================================
--- coregrind/m_syswrap/syswrap-linux.c	(revision 15922)
+++ coregrind/m_syswrap/syswrap-linux.c	(working copy)
@@ -7082,7 +7082,8 @@
       break;
 
 #  if defined(VGPV_arm_linux_android) || defined(VGPV_x86_linux_android) \
-      || defined(VGPV_mips32_linux_android)
+      || defined(VGPV_mips32_linux_android) \
+      || defined(VGPV_arm64_linux_android)
    /* ashmem */
    case VKI_ASHMEM_GET_SIZE:
    case VKI_ASHMEM_SET_SIZE:
@@ -9574,7 +9575,8 @@
       break;
 
 #  if defined(VGPV_arm_linux_android) || defined(VGPV_x86_linux_android) \
-      || defined(VGPV_mips32_linux_android)
+      || defined(VGPV_mips32_linux_android) \
+      || defined(VGPV_arm64_linux_android)
    /* ashmem */
    case VKI_ASHMEM_GET_SIZE:
    case VKI_ASHMEM_SET_SIZE:
Index: include/vki/vki-linux.h
===================================================================
--- include/vki/vki-linux.h	(revision 15922)
+++ include/vki/vki-linux.h	(working copy)
@@ -3009,7 +3009,8 @@
 //----------------------------------------------------------------------
 
 #if defined(VGPV_arm_linux_android) || defined(VGPV_x86_linux_android) \
-    || defined(VGPV_mips32_linux_android)
+    || defined(VGPV_mips32_linux_android) \
+    || defined(VGPV_arm64_linux_android)
 
 #define VKI_ASHMEM_NAME_LEN 256
Comment 4 Anton Kirilov 2016-08-04 14:44:06 UTC
I have tested the patch, and it makes the error message disappear.
Comment 5 Julian Seward 2016-08-04 21:12:00 UTC
Fixed, valgrind r15923.  Thanks for the test.