Fix card mark stealing issue (#51104)

Fix issue with card marking stealing where getting a new chunk caused the "card" variable to go backwards. This caused an extra call to card_transition, which in turn caused cards to be cleared that should be set. The ultimate result is memory corruption.

Here are the conditions that cause the bug to surface:
- an object containing pointers that straddles a 2 MB boundary
- the objects contains just value types (i.e. no pointers) for at least 256 bytes after the 2 MB boundary
- there is a generation-crossing pointer afterwards in the same object
- but that is the only generation-crossing pointer in that 256 byte range

The bug comes about because of the following sequence of events:
- in mark_through_cards_for_segments, we scan an object at the end of a 2 MB chunk
- we encounter a pointer location that is already outside of that chunk (it belongs to card 2 in the next chunk)
- we call card_transition, which advances the card to the card of the pointer location (i.e., to card 2 in the next chunk)
- we realize that we need to get a new chunk
- when we get the chunk, we set the card to card 0 in the chunk
- we return to mark_through_cards_for_segments
- as the next chunk is adjacent to the current one, we continue processing the current object
- when we encounter the next pointer location in the object, we trigger a card_transition again
- this will erroneously not clear cards 0 and 1 because we think it there is a cross-gen pointer
- it will also reset the cross-gen generation pointer counter
- thus, if there are no other cross-generation pointers in the 256 bytes described by card 2, card 2 will be erroneously cleared
- having cards cleared erroneously ultimately leads to heap corruption

The fix simply makes sure the "card" variable doesn't go backwards in find_next_chunk.

It fixes the issue because it avoids the double card_transition.

It is safe because during iteration of a segment, the "card" variable must always increase. When we switch to another segment, it may decrease, but that is fine because in this case it will be re-initialized by the logic in mark_through_cards_segments.

1 files changed, 1 insertions, 1 deletions

diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp
index 0ec1c76ffaa..4437db322e3 100644
--- a/src/coreclr/gc/gc.cpp
+++ b/src/coreclr/gc/gc.cpp
@@ -33654,7 +33654,7 @@ bool gc_heap::find_next_chunk(card_marking_enumerator& card_mark_enumerator, hea
             dprintf (3, ("No more chunks on heap %d\n", heap_number));
             return false;
         }
-        card = card_of (chunk_low);
+        card = max(card, card_of(chunk_low));
         card_word_end = (card_of(align_on_card_word(chunk_high)) / card_word_width);
         dprintf (3, ("Moved to next chunk on heap %d: [%Ix,%Ix[", heap_number, (size_t)chunk_low, (size_t)chunk_high));
     }