Broken resetting of subplan hash tables - Mailing list pgsql-hackers

From Andreas Karlsson
Subject Broken resetting of subplan hash tables
Date
Msg-id edb62547-c453-c35b-3ed6-a069e4d6b937@proxel.se
Whole thread Raw
Responses Re: Broken resetting of subplan hash tables
List pgsql-hackers
Hi,

I looked again at one of the potential issues Ranier Vilela's static 
analysis found and after looking more at it I still think this one is a 
real bug. But my original patch was incorrect and introduced a use after 
free bug.

The code for resetting the hash tables of the SubPlanState node in 
buildSubPlanHash() looks like it can never run, and additionally it 
would be broken if it would ever run. This issue was introduced in 
commit 356687bd825e5ca7230d43c1bffe7a59ad2e77bd.

As far as I gather the following is true:

1. It sets node->hashtable and node->hashnulls to NULL a few lines 
before checking if they are not NULL which means the code for resetting 
them cannot ever be reached.

2. But if we changed to code so that the ResetTupleHashTable() calls are 
reachable we would hit a typo. It resets node->hashtable twice and never 
resets node->hashnulls which would cause non-obvious bugs.

3. Additionally since the memory context used by the hash tables is 
reset in buildSubPlanHash() if we start resetting hash tables we will 
get a use after free bug.

I have attached a patch which makes sure the code for resetting the hash 
tables is actually run while also fixing the code for resetting them.

Since the current behavior of the code in HEAD is not actually broken, 
it is just an optimization which is not used, this fix does not have to 
be backpatched.

Andreas

Attachment

pgsql-hackers by date:

Previous
From: Dilip Kumar
Date:
Subject: Re: Parallel copy
Next
From: Pavel Stehule
Date:
Subject: Re: proposal: schema variables