Possible cache reference leak by removeExtObjInitPriv
Hello.
Recently a cache reference leak was reported then fixed [1]/messages/by-id/BYAPR08MB5606D1453D7F50E2AF4D2FD29AD80@BYAPR08MB5606.namprd08.prod.outlook.com.
I happened to notice a similar possible leakage in
removeEtObjInitPriv. I haven't found a way to reach the code, but can
be forcibly caused by tweaking the condition.
Please find the attached.
regards.
[1]: /messages/by-id/BYAPR08MB5606D1453D7F50E2AF4D2FD29AD80@BYAPR08MB5606.namprd08.prod.outlook.com
Attachments:
fix_aclchk_cacheref_leak-1.patchtext/x-patch; charset=us-asciiDownload
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index a3f680d388..11d9de9031 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -5869,11 +5869,17 @@ removeExtObjInitPriv(Oid objoid, Oid classoid)
/* Indexes don't have permissions */
if (pg_class_tuple->relkind == RELKIND_INDEX ||
pg_class_tuple->relkind == RELKIND_PARTITIONED_INDEX)
+ {
+ ReleaseSysCache(tuple);
return;
+ }
/* Composite types don't have permissions either */
if (pg_class_tuple->relkind == RELKIND_COMPOSITE_TYPE)
+ {
+ ReleaseSysCache(tuple);
return;
+ }
/*
* If this isn't a sequence, index, or composite type then it's
Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
Recently a cache reference leak was reported then fixed [1].
I happened to notice a similar possible leakage in
removeEtObjInitPriv. I haven't found a way to reach the code, but can
be forcibly caused by tweaking the condition.
Please find the attached.
Ugh. recordExtObjInitPriv has the same problem.
I wonder whether there is any way to teach Coverity, or some other
static analyzer, to look for code paths that leak cache refcounts.
It seems isomorphic to detecting memory leaks, which Coverity is
reasonably good at.
regards, tom lane
At Fri, 17 Apr 2020 13:07:15 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in
Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
Recently a cache reference leak was reported then fixed [1].
I happened to notice a similar possible leakage in
removeEtObjInitPriv. I haven't found a way to reach the code, but can
be forcibly caused by tweaking the condition.
Please find the attached.Ugh. recordExtObjInitPriv has the same problem.
Thanks for commit it.
I wonder whether there is any way to teach Coverity, or some other
static analyzer, to look for code paths that leak cache refcounts.
It seems isomorphic to detecting memory leaks, which Coverity is
reasonably good at.
Indeed.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center