From: Philippe Waroquiers Date: Sun, 11 Mar 2012 17:59:00 +0000 (+0000) Subject: Ensure VALGRIND_MALLOCLIKE_BLOCK protects the red zones. X-Git-Tag: svn/VALGRIND_3_8_0~414 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=825fdcf3cb833ee4149b985380b9f092a861ff71;p=thirdparty%2Fvalgrind.git Ensure VALGRIND_MALLOCLIKE_BLOCK protects the red zones. * Redzones for custom alloc were not protected by VALGRIND_MALLOCLIKE_BLOCK. mc_main.c client request handling completed with protection of the redzones. * custom_alloc.c test modified to test this case. * mc_errors.c modified so as to first search for a malloc-ed block bracketting the error : for a custom allocator, a recently freed block can have just been re-allocated. In such a case, describing the address (e.g. in case of error) points to the block freed rather than to the block just allocated. If there is *also* a recently freed block bracketting the address, the block description is changed to indicate that. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@12439 --- diff --git a/NEWS b/NEWS index 9e8d0c5248..f2a65c797b 100644 --- a/NEWS +++ b/NEWS @@ -22,6 +22,10 @@ Release 3.8.0 (????) - Addition of GDB server monitor command 'who_points_at' that lists the locations pointing at a block. + - if a redzone size > 0 is given, VALGRIND_MALLOCLIKE_BLOCK now + will detect an invalid access of these redzones, by marking them + noaccess. + * ==================== OTHER CHANGES ==================== * The C++ demangler has been updated so as to work well with C++ diff --git a/memcheck/mc_errors.c b/memcheck/mc_errors.c index 7c09804f2f..5084f2853d 100644 --- a/memcheck/mc_errors.c +++ b/memcheck/mc_errors.c @@ -1115,30 +1115,41 @@ static void describe_addr ( Addr a, /*OUT*/AddrInfo* ai ) if (mempool_block_maybe_describe( a, ai )) { return; } - /* -- Search for a recently freed block which might bracket it. -- */ - mc = MC_(get_freed_block_bracketting)( a ); - if (mc) { - ai->tag = Addr_Block; - ai->Addr.Block.block_kind = Block_Freed; - ai->Addr.Block.block_desc = "block"; - ai->Addr.Block.block_szB = mc->szB; - ai->Addr.Block.rwoffset = (Word)a - (Word)mc->data; - ai->Addr.Block.lastchange = mc->where; - return; - } + /* Blocks allocated by memcheck malloc functions are either + on the recently freed list or on the malloc-ed list. + Custom blocks can be on both : a recently freed block might + have been just re-allocated. + So, first search the malloc-ed block, as the most recent + block is the probable cause of error. + We however detect and report that this is a recently re-allocated + block. */ /* -- Search for a currently malloc'd block which might bracket it. -- */ VG_(HT_ResetIter)(MC_(malloc_list)); while ( (mc = VG_(HT_Next)(MC_(malloc_list))) ) { if (addr_is_in_MC_Chunk_default_REDZONE_SZB(mc, a)) { ai->tag = Addr_Block; ai->Addr.Block.block_kind = Block_Mallocd; - ai->Addr.Block.block_desc = "block"; + if (MC_(get_freed_block_bracketting)( a )) + ai->Addr.Block.block_desc = "recently re-allocated block"; + else + ai->Addr.Block.block_desc = "block"; ai->Addr.Block.block_szB = mc->szB; ai->Addr.Block.rwoffset = (Word)a - (Word)mc->data; ai->Addr.Block.lastchange = mc->where; return; } } + /* -- Search for a recently freed block which might bracket it. -- */ + mc = MC_(get_freed_block_bracketting)( a ); + if (mc) { + ai->tag = Addr_Block; + ai->Addr.Block.block_kind = Block_Freed; + ai->Addr.Block.block_desc = "block"; + ai->Addr.Block.block_szB = mc->szB; + ai->Addr.Block.rwoffset = (Word)a - (Word)mc->data; + ai->Addr.Block.lastchange = mc->where; + return; + } /* -- Perhaps the variable type/location data describes it? -- */ ai->Addr.Variable.descr1 = VG_(newXA)( VG_(malloc), "mc.da.descr1", diff --git a/memcheck/mc_main.c b/memcheck/mc_main.c index 12f6f292ed..ccad3bc351 100644 --- a/memcheck/mc_main.c +++ b/memcheck/mc_main.c @@ -5553,11 +5553,15 @@ static Bool mc_handle_client_request ( ThreadId tid, UWord* arg, UWord* ret ) case VG_USERREQ__MALLOCLIKE_BLOCK: { Addr p = (Addr)arg[1]; SizeT sizeB = arg[2]; - //UInt rzB = arg[3]; XXX: unused! + UInt rzB = arg[3]; Bool is_zeroed = (Bool)arg[4]; MC_(new_block) ( tid, p, sizeB, /*ignored*/0, is_zeroed, MC_AllocCustom, MC_(malloc_list) ); + if (rzB > 0) { + MC_(make_mem_noaccess) ( p - rzB, rzB); + MC_(make_mem_noaccess) ( p + sizeB, rzB); + } return True; } case VG_USERREQ__RESIZEINPLACE_BLOCK: { diff --git a/memcheck/tests/custom_alloc.c b/memcheck/tests/custom_alloc.c index 2f91c6fe87..56cf94b274 100644 --- a/memcheck/tests/custom_alloc.c +++ b/memcheck/tests/custom_alloc.c @@ -54,6 +54,44 @@ static void custom_free(void* p) VALGRIND_FREELIKE_BLOCK( p, RZ ); } +static void checkredzone(void) +{ + /* check that accessing the redzone of a MALLOCLIKE block + is detected when the superblock was not marked as no access. */ + char superblock[1 + RZ + 20 + RZ + 1]; + char *p = 1 + RZ + superblock; + assert(RZ > 0); + + // Indicate we have allocated p from our superblock: + VALGRIND_MALLOCLIKE_BLOCK( p, 20, RZ, /*is_zeroed*/1 ); + p[0] = 0; + p[-1] = p[0]; // error expected + p[-RZ] = p[0]; // error expected + p[-RZ-1] = p[0]; // no error expected + + p[19] = 0; + p[19 + 1] = p[0]; // error expected + p[19 + RZ] = p[0]; // error expected + p[19 + RZ + 1] = p[0]; // no error expected + + VALGRIND_FREELIKE_BLOCK( p, RZ ); + + // Now, indicate we have re-allocated p from our superblock + // but with only a size 10. + VALGRIND_MALLOCLIKE_BLOCK( p, 10, RZ, /*is_zeroed*/1 ); + p[0] = 0; + p[-1] = p[0]; // error expected + p[-RZ] = p[0]; // error expected + p[-RZ-1] = p[0]; // no error expected + + p[9] = 0; + p[9 + 1] = p[0]; // error expected + p[9 + RZ] = p[0]; // error expected + p[9 + RZ + 1] = p[0]; // no error expected + + VALGRIND_FREELIKE_BLOCK( p, RZ ); + +} @@ -104,16 +142,14 @@ int main(void) make_leak(); x = array[0]; // use after free (ok without MALLOCLIKE/MAKE_MEM_NOACCESS) - // (nb: initialised because is_zeroed==1 above) - // unfortunately not identified as being in a free'd - // block because the freeing of the block and shadow - // chunk isn't postponed. // Bug 137073: passing 0 to MALLOCLIKE_BLOCK was causing an assertion // failure. Test for this (and likewise for FREELIKE_BLOCK). VALGRIND_MALLOCLIKE_BLOCK(0,0,0,0); VALGRIND_FREELIKE_BLOCK(0,0); - + + checkredzone(); + return x; // leak from make_leak() diff --git a/memcheck/tests/custom_alloc.stderr.exp b/memcheck/tests/custom_alloc.stderr.exp index bb55f8c0cf..fcff3ec5f5 100644 --- a/memcheck/tests/custom_alloc.stderr.exp +++ b/memcheck/tests/custom_alloc.stderr.exp @@ -1,45 +1,108 @@ Invalid write of size 4 - at 0x........: main (custom_alloc.c:79) + at 0x........: main (custom_alloc.c:117) Address 0x........ is 0 bytes after a block of size 40 alloc'd at 0x........: custom_alloc (custom_alloc.c:47) - by 0x........: main (custom_alloc.c:76) + by 0x........: main (custom_alloc.c:114) Invalid write of size 4 - at 0x........: main (custom_alloc.c:83) + at 0x........: main (custom_alloc.c:121) Address 0x........ is 0 bytes after a block of size 20 alloc'd at 0x........: custom_alloc (custom_alloc.c:47) - by 0x........: main (custom_alloc.c:76) + by 0x........: main (custom_alloc.c:114) Conditional jump or move depends on uninitialised value(s) - at 0x........: main (custom_alloc.c:90) + at 0x........: main (custom_alloc.c:128) Invalid write of size 4 - at 0x........: main (custom_alloc.c:93) + at 0x........: main (custom_alloc.c:131) Address 0x........ is 0 bytes after a block of size 28 alloc'd at 0x........: custom_alloc (custom_alloc.c:47) - by 0x........: main (custom_alloc.c:76) + by 0x........: main (custom_alloc.c:114) Invalid free() / delete / delete[] / realloc() - at 0x........: main (custom_alloc.c:96) + at 0x........: main (custom_alloc.c:134) Address 0x........ is 4 bytes inside a block of size 28 alloc'd at 0x........: custom_alloc (custom_alloc.c:47) - by 0x........: main (custom_alloc.c:76) + by 0x........: main (custom_alloc.c:114) Invalid free() / delete / delete[] / realloc() at 0x........: custom_free (custom_alloc.c:54) - by 0x........: main (custom_alloc.c:100) + by 0x........: main (custom_alloc.c:138) Address 0x........ is not stack'd, malloc'd or (recently) free'd Mismatched free() / delete / delete [] at 0x........: custom_free (custom_alloc.c:54) - by 0x........: main (custom_alloc.c:103) + by 0x........: main (custom_alloc.c:141) Address 0x........ is 0 bytes inside a block of size 40 alloc'd at 0x........: malloc (vg_replace_malloc.c:...) - by 0x........: main (custom_alloc.c:102) + by 0x........: main (custom_alloc.c:140) Invalid read of size 4 - at 0x........: main (custom_alloc.c:106) + at 0x........: main (custom_alloc.c:144) Address 0x........ is 0 bytes inside a block of size 28 free'd at 0x........: custom_free (custom_alloc.c:54) - by 0x........: main (custom_alloc.c:98) + by 0x........: main (custom_alloc.c:136) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:68) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 1 bytes before a block of size 20 alloc'd + at 0x........: checkredzone (custom_alloc.c:66) + by 0x........: main (custom_alloc.c:151) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:69) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 8 bytes before a block of size 20 alloc'd + at 0x........: checkredzone (custom_alloc.c:66) + by 0x........: main (custom_alloc.c:151) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:73) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 0 bytes after a block of size 20 alloc'd + at 0x........: checkredzone (custom_alloc.c:66) + by 0x........: main (custom_alloc.c:151) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:74) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 7 bytes after a block of size 20 alloc'd + at 0x........: checkredzone (custom_alloc.c:66) + by 0x........: main (custom_alloc.c:151) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:83) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 1 bytes before a recently re-allocated block of size 10 alloc'd + at 0x........: checkredzone (custom_alloc.c:81) + by 0x........: main (custom_alloc.c:151) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:84) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 8 bytes before a recently re-allocated block of size 10 alloc'd + at 0x........: checkredzone (custom_alloc.c:81) + by 0x........: main (custom_alloc.c:151) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:88) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 0 bytes after a recently re-allocated block of size 10 alloc'd + at 0x........: checkredzone (custom_alloc.c:81) + by 0x........: main (custom_alloc.c:151) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:89) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 7 bytes after a recently re-allocated block of size 10 alloc'd + at 0x........: checkredzone (custom_alloc.c:81) + by 0x........: main (custom_alloc.c:151) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:90) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 8 bytes after a recently re-allocated block of size 10 alloc'd + at 0x........: checkredzone (custom_alloc.c:81) + by 0x........: main (custom_alloc.c:151) diff --git a/memcheck/tests/custom_alloc.stderr.exp-s390x-mvc b/memcheck/tests/custom_alloc.stderr.exp-s390x-mvc index 70dbd27355..6618a8946c 100644 --- a/memcheck/tests/custom_alloc.stderr.exp-s390x-mvc +++ b/memcheck/tests/custom_alloc.stderr.exp-s390x-mvc @@ -1,45 +1,108 @@ Invalid write of size 4 - at 0x........: main (custom_alloc.c:79) + at 0x........: main (custom_alloc.c:117) Address 0x........ is 0 bytes after a block of size 40 alloc'd at 0x........: custom_alloc (custom_alloc.c:47) - by 0x........: main (custom_alloc.c:76) + by 0x........: main (custom_alloc.c:114) Invalid write of size 4 - at 0x........: main (custom_alloc.c:83) + at 0x........: main (custom_alloc.c:121) Address 0x........ is 0 bytes after a block of size 20 alloc'd at 0x........: custom_alloc (custom_alloc.c:47) - by 0x........: main (custom_alloc.c:76) + by 0x........: main (custom_alloc.c:114) Conditional jump or move depends on uninitialised value(s) - at 0x........: main (custom_alloc.c:90) + at 0x........: main (custom_alloc.c:128) Invalid write of size 4 - at 0x........: main (custom_alloc.c:93) + at 0x........: main (custom_alloc.c:131) Address 0x........ is 0 bytes after a block of size 28 alloc'd at 0x........: custom_alloc (custom_alloc.c:47) - by 0x........: main (custom_alloc.c:76) + by 0x........: main (custom_alloc.c:114) Invalid free() / delete / delete[] / realloc() - at 0x........: main (custom_alloc.c:96) + at 0x........: main (custom_alloc.c:134) Address 0x........ is 4 bytes inside a block of size 28 alloc'd at 0x........: custom_alloc (custom_alloc.c:47) - by 0x........: main (custom_alloc.c:76) + by 0x........: main (custom_alloc.c:114) Invalid free() / delete / delete[] / realloc() at 0x........: custom_free (custom_alloc.c:54) - by 0x........: main (custom_alloc.c:100) + by 0x........: main (custom_alloc.c:138) Address 0x........ is not stack'd, malloc'd or (recently) free'd Mismatched free() / delete / delete [] at 0x........: custom_free (custom_alloc.c:54) - by 0x........: main (custom_alloc.c:103) + by 0x........: main (custom_alloc.c:141) Address 0x........ is 0 bytes inside a block of size 40 alloc'd at 0x........: malloc (vg_replace_malloc.c:...) - by 0x........: main (custom_alloc.c:102) + by 0x........: main (custom_alloc.c:140) Invalid read of size 1 - at 0x........: main (custom_alloc.c:106) + at 0x........: main (custom_alloc.c:144) Address 0x........ is 0 bytes inside a block of size 28 free'd at 0x........: custom_free (custom_alloc.c:54) - by 0x........: main (custom_alloc.c:98) + by 0x........: main (custom_alloc.c:136) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:68) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 1 bytes before a block of size 20 alloc'd + at 0x........: checkredzone (custom_alloc.c:66) + by 0x........: main (custom_alloc.c:151) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:69) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 8 bytes before a block of size 20 alloc'd + at 0x........: checkredzone (custom_alloc.c:66) + by 0x........: main (custom_alloc.c:151) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:73) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 0 bytes after a block of size 20 alloc'd + at 0x........: checkredzone (custom_alloc.c:66) + by 0x........: main (custom_alloc.c:151) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:74) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 7 bytes after a block of size 20 alloc'd + at 0x........: checkredzone (custom_alloc.c:66) + by 0x........: main (custom_alloc.c:151) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:83) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 1 bytes before a recently re-allocated block of size 10 alloc'd + at 0x........: checkredzone (custom_alloc.c:81) + by 0x........: main (custom_alloc.c:151) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:84) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 8 bytes before a recently re-allocated block of size 10 alloc'd + at 0x........: checkredzone (custom_alloc.c:81) + by 0x........: main (custom_alloc.c:151) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:88) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 0 bytes after a recently re-allocated block of size 10 alloc'd + at 0x........: checkredzone (custom_alloc.c:81) + by 0x........: main (custom_alloc.c:151) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:89) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 7 bytes after a recently re-allocated block of size 10 alloc'd + at 0x........: checkredzone (custom_alloc.c:81) + by 0x........: main (custom_alloc.c:151) + +Invalid write of size 1 + at 0x........: checkredzone (custom_alloc.c:90) + by 0x........: main (custom_alloc.c:151) + Address 0x........ is 8 bytes after a recently re-allocated block of size 10 alloc'd + at 0x........: checkredzone (custom_alloc.c:81) + by 0x........: main (custom_alloc.c:151)