projects / postgresql.git / commitdiff
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: ff9d757)
Fix dangling smgr_owner pointer when a fake relcache entry is freed.
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>
Fri, 7 Mar 2014 11:25:11 +0000 (13:25 +0200)
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>
Fri, 7 Mar 2014 11:43:28 +0000 (13:43 +0200)
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.

src/backend/access/transam/xlogutils.c patch | blob | blame | history
src/backend/storage/smgr/smgr.c patch | blob | blame | history
src/include/storage/smgr.h patch | blob | blame | history

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 1811c91d5811bc0fe1ae928986a93d4fed9f550a..8f0bbc21dbb0a1562232896cd7038cb427907649 100644 (file)
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -413,6 +413,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 7d1cd5af1688f11cfae629227ba5f9b87f728404..9379e3379309d8d2f22f566a2719754ac53137a6 100644 (file)
--- a/src/backend/storage/smgr/smgr.c
+++ b/src/backend/storage/smgr/smgr.c
@@ -193,6 +193,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 1761f1c80894cf0a631db34731c3366c45a595c6..957f2108d5426e6c440ee757c49e948d25b29062 100644 (file)
--- a/src/include/storage/smgr.h
+++ b/src/include/storage/smgr.h
@@ -62,6 +62,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);
This is the main PostgreSQL git repository.