From: Andrey Lepikhov <a.lepikhov@postgrespro.ru>
Subject: Insecure initialization of required_relids field
Organization: Postgres Professional
To: PostgreSQL-Dev <pgsql-hackers@postgresql.org>
Message-ID: <5c21029d-81a2-c999-6744-6a898fcc9a19@postgrespro.ru>
Date: Mon, 15 Jul 2019 11:12:35 +0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
MIME-Version: 1.0
Content-Type: multipart/mixed;
 boundary="------------733A58098BF09C1863AF9B05"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------733A58098BF09C1863AF9B05
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

Hi,

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).

[1] commit 4e97631e6a9, analyzejoins.c, line 434,435:
rinfo->required_relids = bms_copy(rinfo->required_relids);
rinfo->required_relids = bms_del_member(rinfo->required_relids, relid);
[2] https://commitfest.postgresql.org/23/1712/

-- 
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company

--------------733A58098BF09C1863AF9B05
Content-Type: text/x-patch;
 name="0001-More-secure-initialization-of-required_relids-field.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename*0="0001-More-secure-initialization-of-required_relids-field.pat";
 filename*1="ch"

From 1dc021c0c9827ecd530ee3bb0020bad9bc5d14b6 Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" <a.lepikhov@postgrespro.ru>
Date: Mon, 15 Jul 2019 11:07:20 +0500
Subject: [PATCH] More secure initialization of required_relids field

---
 src/backend/optimizer/util/restrictinfo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c
index 3b50fd29ad..450558e61b 100644
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -172,7 +172,7 @@ make_restrictinfo_internal(Expr *clause,
 	if (required_relids != NULL)
 		restrictinfo->required_relids = required_relids;
 	else
-		restrictinfo->required_relids = restrictinfo->clause_relids;
+		restrictinfo->required_relids = bms_copy(restrictinfo->clause_relids);
 
 	/*
 	 * Fill in all the cacheable fields with "not yet set" markers. None of
-- 
2.17.1


--------------733A58098BF09C1863AF9B05--