Hi all,
TransactionIdGetCommitTsData@commit_ts.c does the following:
if (ts)
*ts = entry.time;
[...]
return *ts != 0;
This is a bad idea, because if TransactionIdGetCommitTsData is called
with ts == NULL this would simply crash. It seems to me that it makes
little sense to have ts == NULL either way because this function is
intended to return a timestamp in any case, hence I think that we
should simply Assert(ts == NULL) and remove those checks.
Petr, Alvaro, perhaps you intended something like the patch attached,
or perhaps something like that? This would be useful to not ERROR
should an OOM happen when allocating a timestamp pointer, though I
doubt that this is the first intention:
return ts != NULL ? *ts != 0 : false;
Regards,
--
Michael
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 5ad35c0..d35da03 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -266,6 +266,8 @@ TransactionIdGetCommitTsData(TransactionId xid, TimestampTz *ts,
TransactionId oldestCommitTs;
TransactionId newestCommitTs;
+ Assert(ts != NULL);
+
/* Error if module not enabled */
if (!track_commit_timestamp)
ereport(ERROR,
@@ -294,8 +296,7 @@ TransactionIdGetCommitTsData(TransactionId xid, TimestampTz *ts,
TransactionIdPrecedes(xid, oldestCommitTs) ||
TransactionIdPrecedes(newestCommitTs, xid))
{
- if (ts)
- *ts = 0;
+ *ts = 0;
if (nodeid)
*nodeid = InvalidRepOriginId;
return false;
@@ -312,8 +313,7 @@ TransactionIdGetCommitTsData(TransactionId xid, TimestampTz *ts,
LWLockAcquire(CommitTsLock, LW_SHARED);
if (commitTsShared->xidLastCommit == xid)
{
- if (ts)
- *ts = commitTsShared->dataLastCommit.time;
+ *ts = commitTsShared->dataLastCommit.time;
if (nodeid)
*nodeid = commitTsShared->dataLastCommit.nodeid;
@@ -330,8 +330,8 @@ TransactionIdGetCommitTsData(TransactionId xid, TimestampTz *ts,
SizeOfCommitTimestampEntry * entryno,
SizeOfCommitTimestampEntry);
- if (ts)
- *ts = entry.time;
+ *ts = entry.time;
+
if (nodeid)
*nodeid = entry.nodeid;
diff --git a/src/bin/pg_dump/pg_backup_tar.c b/src/bin/pg_dump/pg_backup_tar.c
index 8730c5e..3724d7e 100644
--- a/src/bin/pg_dump/pg_backup_tar.c
+++ b/src/bin/pg_dump/pg_backup_tar.c
@@ -379,8 +379,16 @@ tarOpen(ArchiveHandle *AH, const char *filename, char mode)
}
else
{
+ int old_umask;
+
tm = pg_malloc0(sizeof(TAR_MEMBER));
+ /*
+ * For security, no temporary file created can be group or other
+ * accessible.
+ */
+ old_umask = umask(S_IRWXG | S_IRWXO);
+
#ifndef WIN32
tm->tmpFH = tmpfile();
#else
@@ -415,6 +423,8 @@ tarOpen(ArchiveHandle *AH, const char *filename, char mode)
if (tm->tmpFH == NULL)
exit_horribly(modulename, "could not generate temporary file name: %s\n", strerror(errno));
+ umask(old_umask);
+
#ifdef HAVE_LIBZ
if (AH->compression != 0)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
