aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>2014-03-07 13:25:11 +0200
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>2014-03-07 13:43:28 +0200
commitd411d6eff1b7e91de0fecece08acc6b6eca04d6d (patch)
tree7526ef4f87c6d66d29aa2184bf6e563bf9d26a93
parentff9d757c67786b389f19abc3187d158adda500d8 (diff)
downloadpostgresql-d411d6eff1b7e91de0fecece08acc6b6eca04d6d.tar.gz
postgresql-d411d6eff1b7e91de0fecece08acc6b6eca04d6d.zip
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.
-rw-r--r--src/backend/access/transam/xlogutils.c3
-rw-r--r--src/backend/storage/smgr/smgr.c18
-rw-r--r--src/include/storage/smgr.h1
3 files changed, 22 insertions, 0 deletions
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 1811c91d581..8f0bbc21dbb 100644
--- 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 7d1cd5af168..9379e337930 100644
--- a/src/backend/storage/smgr/smgr.c
+++ b/src/backend/storage/smgr/smgr.c
@@ -194,6 +194,24 @@ smgrsetowner(SMgrRelation *owner, SMgrRelation 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?
*/
bool
diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h
index 1761f1c8089..957f2108d54 100644
--- 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);