ASSERT (base_unitttests.exe xpsp3) drmemory/readwrite.c:2564: addr + sz > addr (address overflow)

[derekbruening]

From derek.br...@gmail.com on August 26, 2010 09:56:56

base_unittests.exe from issue #251 running on xpsp3 hits an assert:

[----------] 4 tests from WMIUtilTest
[ RUN ] WMIUtilTest.TestLocalConnectionSecurityBlanket
:::Dr.Memory::: ASSERT FAILURE (thread 134248): E:/derek/dr/win32/oss-clients/drmemory/drmemory/readwrite.c:2564: addr + sz > addr (address overflow)

size from a PORT_MESSAGE is -1:

0:001> dt -b pm
Local var @ 0x24529dd4 Type _PORT_MESSAGE
+0x000 u1 :
+0x000 s1 :
+0x000 DataLength : -1
+0x002 TotalLength : -1
+0x000 Length : 0xffffffff
+0x004 u2 :
+0x000 s2 :
+0x000 Type : 0
+0x002 DataInfoOffset : 0
+0x000 ZeroInit : 0
+0x008 ClientId :
+0x000 UniqueProcess : 0x00040023
+0x004 UniqueThread : 0x000a01ca
+0x008 DoNotUseThisField : 1.3916430357687966e-308
+0x010 MessageId : 0xfe
+0x014 u3 :
+0x000 ClientViewSize : 0x1c9048
+0x000 CallbackId : 0x1c9048

Original issue: http://code.google.com/p/drmemory/issues/detail?id=35

[derekbruening]

From timurrrr@google.com on August 27, 2010 03:58:53

On my machine with AV, it hits the issue #23 assert.
On the Chromium build bot with no AV all "WMI*" tests pass on r39 without assertions.

[derekbruening]

From timurrrr@google.com on August 27, 2010 04:03:34

"size" in handle_port_message is "uint".

Probably, the following patch should fix the issue
(though I'm not sure what Length=-1 means. Error?)

diff --git drmemory/syscall_windows.c drmemory/syscall_windows.c
index 44f3119..ddd1dec 100644
--- drmemory/syscall_windows.c
+++ drmemory/syscall_windows.c
@@ -579,11 +579,14 @@ static bool handle_port_message_access(int sysnum, dr_mcontext_t *mc,
size = pm.u1.s1.TotalLength;
else
size = pm.u1.Length;

•    if (size < sizeof(pm))
  

•    if (size == (uint)-1)
  

•        size = 0;
  

•    else if (size < sizeof(pm))
       size = sizeof(pm);
   LOG(2, "total size of PORT_MESSAGE arg %d is %d\n", arg_num, size);
  

  }
• check_sysmem(check_type, sysnum, start, size, mc, NULL);
• if (size > 0)

•    check_sysmem(check_type, sysnum, start, size, mc, NULL);
  

  return true;
  }

[derekbruening]

From derek.br...@gmail.com on August 29, 2010 13:59:36

no, the problem is more fundamental, with OUT PORT_MESSAGE args being completely uninit, which I did not realize: there is a max size.

fixed in r41 :

• Issue 35 : split pre and post checks properly w/ addressable
  on pre and write on post if call succeeds.
  For PORT_MESSAGE consider write param as max size uninit.

Status: Verified