Re: Crashing in _Block_release [Fixed?]

[Jens Alfke]

On Feb 29, 2012, at 5:34 PM, I wrote:

==13971== Invalid read of size 4
==13971==    at 0x4A88075: _Block_object_dispose (blocks_runtime.m:206)
==13971==    by 0x47B0F28: __destroy_helper_block_ (MYBlockUtils.m:54)
==13971==    by 0x4A88334: _Block_release (blocks_runtime.m:299)
==13971==    by 0x414476B: _i_GSBlock__release (GSBlocks.m:81)

My guess is that, even though _Block_object_dispose casts its input pointer to a block_byref_obj, not all the structures passed to it are the full size of a block_byref_obj. The structure has its own ‘size’ field after all. So there seems to be a missing test here, for whether the object passed in actually contains a byref_dispose field before dereferencing that field.

I looked for other uses of byref_dispose, and some of them check the size field first, so I added that to the problematic bit of code. The crash and the valgrind warnings went away. So is this a correct fix? (I notice that slightly later, on line 215, there’s a different test, of the BLOCK_HAS_COPY_DISPOSE flag. Would that be better to do?)

—Jens

Index: blocks_runtime.m

===================================================================

--- blocks_runtime.m (revision 34837)

+++ blocks_runtime.m (working copy)

@@ -203,7 +203,7 @@

  int refcount = (src->flags & BLOCK_REFCOUNT_MASK) == 0 ? 0 : decrement24(&src->flags);

  if (refcount == 0)

  {

- if (0 != src->byref_dispose)

+ if((size_t)src->size >= sizeof(struct block_byref_obj) && (0 != src->byref_dispose))

  {

  src->byref_dispose(src);

  }

smime.p7s
Description: S/MIME cryptographic signature