facebook/redex

use-after-delete in Reachability.cpp

justhecuke opened this issue · 4 comments

sweep will eventually call DexField::delete_field_DO_NOT_USE to permanently delete DexField pointers. Unfortunately, I don't think the code also removes the fields from the class that contains it or any other locations which may be holding a copy of the pointer. Because of this, we run into a use-after-delete bug where potentially corrupted memory is read using a copy of a deleted pointer.

I've seen this in the version of code that we use but it's ~1 year old at this point compared to upstream head. From what I've read of the upstream code, however, it seems like the bug is still present. I have no idea how the code works for you, but you must have enough memory to not need to re-use old blocks.

I don't recall running into an issue there. The call to sweep_if_unmarked removes the field from the ifields or sfields list of the class. If the field is referenced somewhere else, it wouldn't be left unmarked. Hopefully this helps.

Hey @justhecuke do you need more assistance on this question please?

I haven't looked into this again for some time and it's not really on my plate anymore. If I do take another look I'll be sure to ask if I need something. The initial investigation was pretty frustrating because the memory corruption made it difficult to know what had changed and lldb's memory read/write breakpoints weren't working well for me.

Sounds good. Closing this issue for now. :)