[PATCH] D146412: [NFC] Fix potential use-after-free in DumpModuleInfoAction::ExecuteAction()

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D146412#4223272 <https://reviews.llvm.org/D146412#4223272>, @Fznamznon wrote:

> In D146412#4220021 <https://reviews.llvm.org/D146412#4220021>, @kastiglione wrote:
>
>> I understand the potential for a use after free, since `OutputStream` is a raw pointer, but I don't understand the fix.
>
> Okay, probably commit message is a little confusing. The potential for a use after free is that `OutputStream` is a raw pointer as you already noted, but it is also accessible outside of `DumpModuleInfoAction::ExecuteAction`. Before the patch there was the case where result of local unique_ptr::get was saved there. When the function is exited, unique_ptr frees the memory it holds, making `OutputStream` poiniting to freed memory. Since `OutputStream` can be easily accessed outside of `DumpModuleInfoAction::ExecuteAction`, hence the potential for use-after-free. I wasn't sure if assigning `nullptr` to `OutputStream` at the end of `DumpModuleInfoAction::ExecuteAction` was a good idea, so I just added code avoiding saving result of unique_ptr::get to `OutputStream` .

Hmmm, but this looks to be the only place `DumpModuleInfoAction` will initialize `OutputStream` to a nonnull value, so that seems to be a pretty significant change in behavior.

Should `OutputStream` be made into a `shared_ptr` so we can clarify the memory ownership instead of leaving it as a raw pointer? That should also address the use-after-free.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146412/new/

https://reviews.llvm.org/D146412