Fix dangling smgr_owner pointer when a fake relcache entry is freed.

A fake relcache entry can "own" a SmgrRelation object, like a regular
relcache entry. But when it was free'd, the owner field in SmgrRelation
was not cleared, so it was left pointing to free'd memory.

Amazingly this apparently hasn't caused crashes in practice, or we would've
heard about it earlier. Andres found this with Valgrind.

Report and fix by Andres Freund, with minor modifications by me. Backpatch
to all supported versions.

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 8ffe1b0f84ba044cd6785190c344c91748a5b8b7..e8c040c63852c826500fa6764d8716dd073f7c26 100644 (file)
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -414,6 +414,9 @@ CreateFakeRelcacheEntry(RelFileNode rnode)
 void
 FreeFakeRelcacheEntry(Relation fakerel)
 {
+       /* make sure the fakerel is not referenced by the SmgrRelation anymore */
+       if (fakerel->rd_smgr != NULL)
+               smgrclearowner(&fakerel->rd_smgr, fakerel->rd_smgr);
        pfree(fakerel);
 }
 

diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c
index 7a35b0a833350b74bd33e5603f2070e7563c7ec0..886176bc69929f3d4baf26187c9d10bfa5f4803a 100644 (file)
--- a/src/backend/storage/smgr/smgr.c
+++ b/src/backend/storage/smgr/smgr.c
@@ -197,6 +197,24 @@ smgrsetowner(SMgrRelation *owner, SMgrRelation reln)
        *owner = reln;
 }
 
+/*
+ * smgrclearowner() -- Remove long-lived reference to an SMgrRelation object
+ *                                        if one exists
+ */
+void
+smgrclearowner(SMgrRelation *owner, SMgrRelation reln)
+{
+       /* Do nothing if the SMgrRelation object is not owned by the owner */
+       if (reln->smgr_owner != owner)
+               return;
+
+       /* unset the owner's reference */
+       *owner = NULL;
+
+       /* unset our reference to the owner */
+       reln->smgr_owner = NULL;
+}
+
 /*
  *     smgrexists() -- Does the underlying file for a fork exist?
  */

diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h
index c037190b4ba9587ee094f93d0a040593beb4e03c..6eee0a44fdf5ff6a91e2351fa8855d1c44573891 100644 (file)
--- a/src/include/storage/smgr.h
+++ b/src/include/storage/smgr.h
@@ -73,6 +73,7 @@ extern void smgrinit(void);
 extern SMgrRelation smgropen(RelFileNode rnode);
 extern bool smgrexists(SMgrRelation reln, ForkNumber forknum);
 extern void smgrsetowner(SMgrRelation *owner, SMgrRelation reln);
+extern void smgrclearowner(SMgrRelation *owner, SMgrRelation reln);
 extern void smgrclose(SMgrRelation reln);
 extern void smgrcloseall(void);
 extern void smgrclosenode(RelFileNode rnode);