Broken resetting of subplan hash tables

From: Andreas Karlsson <andreas(at)proxel(dot)se>
To: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
Subject: Broken resetting of subplan hash tables
Date: 2020-02-29 08:58:46
Message-ID: edb62547-c453-c35b-3ed6-a069e4d6b937@proxel.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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 Content-Type Size
fix-subplan-hash-reset-v2.patch text/x-patch 1.1 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2020-02-29 09:09:47 Re: proposal: schema variables
Previous Message Dilip Kumar 2020-02-29 08:42:50 Re: Parallel copy