Fix use-after-free when pg_hint_plan.parse_messages is ERROR #142
Conversation
| PG_TRY(); | ||
| { | ||
| /* Apply Set hints, then save it as the initial state */ | ||
| setup_guc_enforcement(current_hint_state->set_hints, |
There was a problem hiding this comment.
Hmm.. setup_guc_enforcement() itself has a stack manipulation, and set_config_option_noerror() is written to not throw an error, still we are doing so here. I am not sure what's the best solution is here, but this change makes the stack manipulation more complicated than it ought to be, IMO.
There was a problem hiding this comment.
I can confirm that the patch is incorrect. Actually, any ERROR happening between the point where we have push_hint() up to the point where we enter in the TRY/CATCH block would cause an incorrect stack, and there can be much more that could trigger a corrupted stack. Just to mention one: out-of-memory failures. For instance, if a palloc() fails between the push_hint() and the TRY/CATCH block, we're out. I think that we should enlarge the window where we use the TRY/CATCH block instead, beginning it just after the hints are pushed on the stack.
This pull request resolves issue #141