From: Andrey Lepikhov <a.lepikhov@postgrespro.ru>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL-Dev <pgsql-hackers@postgresql.org>
Subject: Re: Insecure initialization of required_relids field
Date: Mon, 15 Jul 2019 22:08:41 +0500
Message-ID: <b8e42289-1b29-8d59-dd8c-93e0584d8946@postgrespro.ru> (raw)
In-Reply-To: <29956.1563198507@sss.pgh.pa.us>
References: <5c21029d-81a2-c999-6744-6a898fcc9a19@postgrespro.ru>
<29956.1563198507@sss.pgh.pa.us>
On 15/07/2019 18:48, Tom Lane wrote:
> Andrey Lepikhov <a.lepikhov@postgrespro.ru> writes:
>> commit a31ad27fc5d introduced required_relids field. By default, it
>> links to the clause_relids.
>> It works good while we do not modify clause_relids or required_relids.
>> But in the case of modification such initialization demands us to
>> remember, that this field is shared. And we need to do bms_copy() before
>> making any changes (see [1] for example).
>> Also, we make some changes of the RestrictInfo fields (see patch [2])
>> during removing of unneeded self joins.
>> I propose to do more secure initialization way of required_relids (see
>> patch in attachment).
>
> This seems fairly expensive (which is why it wasn't done like that
> to start with) and you've pointed to no specific bug that it fixes.
> Seeing that (a) the original commit is 14 years old, and (b) changing
> either of these fields after-the-fact is at most a very niche usage,
> I don't think we really have a problem here.
In the patch 'Removing unneeded self joins' [1] we modify both
clause_relids and required_relids. Valgrind detected a problem: during
the required_relids change routine repalloc() was executed. In this
case, clause_relids will point to free memory block.
In accordance to your answer do you recommend me to make the bms_copy()
call before changing any of clause_relids and required_relids fields?
[1] https://commitfest.postgresql.org/23/1712/
--
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: a.lepikhov@postgrespro.ru, tgl@sss.pgh.pa.us
Subject: Re: Insecure initialization of required_relids field
In-Reply-To: <b8e42289-1b29-8d59-dd8c-93e0584d8946@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox