On 2016-04-03 09:24, Piotr Stefaniak wrote:
from running the regression test suite (including TAP tests) and also
sqlsmith, I've got a couple of places where UBSan reported calls to
memcpy() with null pointer passed as either source or destination.
Patch attached.
Patch updated.
Since this time the patch includes fixes for other standard library
function calls (memset and bsearch), I'm renaming the patch file to be
more generic.
commit 75e849e83c8f7d6b4caab13271b7b0fcf124d497
Author: Piotr Stefaniak <postg...@piotr-stefaniak.me>
Date: Sat Apr 16 14:28:34 2016 +0200
Don't pass null pointers to functions that require the pointers to be non null.
diff --git a/contrib/pgcrypto/px.c b/contrib/pgcrypto/px.c
index cfb3b50..600824a 100644
--- a/contrib/pgcrypto/px.c
+++ b/contrib/pgcrypto/px.c
@@ -180,7 +180,7 @@ combo_init(PX_Combo *cx, const uint8 *key, unsigned klen,
memset(ivbuf, 0, ivs);
if (ivlen > ivs)
memcpy(ivbuf, iv, ivs);
- else
+ else if (ivlen > 0)
memcpy(ivbuf, iv, ivlen);
}
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 29fd31a..2ed56ab 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -305,7 +305,7 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
/*
* copy the scan key, if appropriate
*/
- if (key != NULL)
+ if (key != NULL && scan->rs_nkeys > 0)
memcpy(scan->rs_key, key, scan->rs_nkeys * sizeof(ScanKeyData));
/*
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 7e37331..e7599be 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -4872,8 +4872,9 @@ SerializeTransactionState(Size maxsize, char *start_address)
{
if (TransactionIdIsValid(s->transactionId))
workspace[i++] = s->transactionId;
- memcpy(&workspace[i], s->childXids,
- s->nChildXids * sizeof(TransactionId));
+ if (s->nChildXids > 0)
+ memcpy(&workspace[i], s->childXids,
+ s->nChildXids * sizeof(TransactionId));
i += s->nChildXids;
}
Assert(i == nxids);
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index b4dc617..a72795c 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1563,9 +1563,12 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
/* copy running xacts */
sz = sizeof(TransactionId) * builder->running.xcnt_space;
- memcpy(ondisk_c, builder->running.xip, sz);
- COMP_CRC32C(ondisk->checksum, ondisk_c, sz);
- ondisk_c += sz;
+ if (sz > 0)
+ {
+ memcpy(ondisk_c, builder->running.xip, sz);
+ COMP_CRC32C(ondisk->checksum, ondisk_c, sz);
+ ondisk_c += sz;
+ }
/* copy committed xacts */
sz = sizeof(TransactionId) * builder->committed.xcnt;
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 8aa1f49..25ac26f 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -506,12 +506,14 @@ SetTransactionSnapshot(Snapshot sourcesnap, TransactionId sourcexid,
CurrentSnapshot->xmax = sourcesnap->xmax;
CurrentSnapshot->xcnt = sourcesnap->xcnt;
Assert(sourcesnap->xcnt <= GetMaxSnapshotXidCount());
- memcpy(CurrentSnapshot->xip, sourcesnap->xip,
- sourcesnap->xcnt * sizeof(TransactionId));
+ if (sourcesnap->xcnt > 0)
+ memcpy(CurrentSnapshot->xip, sourcesnap->xip,
+ sourcesnap->xcnt * sizeof(TransactionId));
CurrentSnapshot->subxcnt = sourcesnap->subxcnt;
Assert(sourcesnap->subxcnt <= GetMaxSnapshotSubxidCount());
- memcpy(CurrentSnapshot->subxip, sourcesnap->subxip,
- sourcesnap->subxcnt * sizeof(TransactionId));
+ if (sourcesnap->subxcnt > 0)
+ memcpy(CurrentSnapshot->subxip, sourcesnap->subxip,
+ sourcesnap->subxcnt * sizeof(TransactionId));
CurrentSnapshot->suboverflowed = sourcesnap->suboverflowed;
CurrentSnapshot->takenDuringRecovery = sourcesnap->takenDuringRecovery;
/* NB: curcid should NOT be copied, it's a local matter */
diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c
index 0e815a9..0f26bd8 100644
--- a/src/backend/utils/time/tqual.c
+++ b/src/backend/utils/time/tqual.c
@@ -1639,6 +1639,7 @@ HeapTupleHeaderIsOnlyLocked(HeapTupleHeader tuple)
static bool
TransactionIdInArray(TransactionId xid, TransactionId *xip, Size num)
{
+ Assert(xip != NULL);
return bsearch(&xid, xip, num,
sizeof(TransactionId), xidComparator) != NULL;
}
@@ -1675,7 +1676,7 @@ HeapTupleSatisfiesHistoricMVCC(HeapTuple htup, Snapshot snapshot,
return false;
}
/* check if it's one of our txids, toplevel is also in there */
- else if (TransactionIdInArray(xmin, snapshot->subxip, snapshot->subxcnt))
+ else if (snapshot->subxcnt > 0 && TransactionIdInArray(xmin, snapshot->subxip, snapshot->subxcnt))
{
bool resolved;
CommandId cmin = HeapTupleHeaderGetRawCommandId(tuple);
@@ -1717,7 +1718,7 @@ HeapTupleSatisfiesHistoricMVCC(HeapTuple htup, Snapshot snapshot,
return false;
}
/* check if it's a committed transaction in [xmin, xmax) */
- else if (TransactionIdInArray(xmin, snapshot->xip, snapshot->xcnt))
+ else if (snapshot->xcnt > 0 && TransactionIdInArray(xmin, snapshot->xip, snapshot->xcnt))
{
/* fall through */
}
@@ -1750,7 +1751,7 @@ HeapTupleSatisfiesHistoricMVCC(HeapTuple htup, Snapshot snapshot,
}
/* check if it's one of our txids, toplevel is also in there */
- if (TransactionIdInArray(xmax, snapshot->subxip, snapshot->subxcnt))
+ if (snapshot->subxcnt > 0 && TransactionIdInArray(xmax, snapshot->subxip, snapshot->subxcnt))
{
bool resolved;
CommandId cmin;
@@ -1788,7 +1789,7 @@ HeapTupleSatisfiesHistoricMVCC(HeapTuple htup, Snapshot snapshot,
else if (TransactionIdFollowsOrEquals(xmax, snapshot->xmax))
return true;
/* xmax is between [xmin, xmax), check known committed array */
- else if (TransactionIdInArray(xmax, snapshot->xip, snapshot->xcnt))
+ else if (snapshot->xcnt > 0 && TransactionIdInArray(xmax, snapshot->xip, snapshot->xcnt))
return false;
/* xmax is between [xmin, xmax), but known not to have committed yet */
else
diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c
index 1ec74f1..ca0eb7e 100644
--- a/src/fe_utils/print.c
+++ b/src/fe_utils/print.c
@@ -914,7 +914,8 @@ print_aligned_text(const printTableContent *cont, FILE *fout, bool is_pager)
more_col_wrapping = col_count;
curr_nl_line = 0;
- memset(header_done, false, col_count * sizeof(bool));
+ if (col_count > 0)
+ memset(header_done, false, col_count * sizeof(bool));
while (more_col_wrapping)
{
if (opt_border == 2)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
