From b66827ca7c5a4c9e31b1a1eced677f8677efc0cf Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 1 Oct 2018 11:51:07 -0400 Subject: [PATCH] Fix tuple_data_split() to not open a relation without any lock. contrib/pageinspect's tuple_data_split() function thought it could get away with opening the referenced relation with NoLock. In practice there's no guarantee that the current session holds any lock on that rel (even if we just read a page from it), so that this is unsafe. Switch to using AccessShareLock. Also, postpone closing the relation, so that we needn't copy its tupdesc. Also, fix unsafe use of att_isnull() for attributes past the end of the tuple. Per testing with a patch that complains if we open a relation without holding any lock on it. I don't plan to back-patch that patch, but we should close the holes it identifies in all supported branches. Discussion: https://postgr.es/m/2038.1538335244@sss.pgh.pa.us --- contrib/pageinspect/heapfuncs.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c index 7438257c5b..d96ba1e8b6 100644 --- a/contrib/pageinspect/heapfuncs.c +++ b/contrib/pageinspect/heapfuncs.c @@ -298,9 +298,8 @@ tuple_data_split_internal(Oid relid, char *tupdata, TupleDesc tupdesc; /* Get tuple descriptor from relation OID */ - rel = relation_open(relid, NoLock); - tupdesc = CreateTupleDescCopyConstr(rel->rd_att); - relation_close(rel, NoLock); + rel = relation_open(relid, AccessShareLock); + tupdesc = RelationGetDescr(rel); raw_attrs = initArrayResult(BYTEAOID, CurrentMemoryContext, false); nattrs = tupdesc->natts; @@ -317,7 +316,6 @@ tuple_data_split_internal(Oid relid, char *tupdata, bytea *attr_data = NULL; attr = TupleDescAttr(tupdesc, i); - is_null = (t_infomask & HEAP_HASNULL) && att_isnull(i, t_bits); /* * Tuple header can specify less attributes than tuple descriptor as @@ -327,6 +325,8 @@ tuple_data_split_internal(Oid relid, char *tupdata, */ if (i >= (t_infomask2 & HEAP_NATTS_MASK)) is_null = true; + else + is_null = (t_infomask & HEAP_HASNULL) && att_isnull(i, t_bits); if (!is_null) { @@ -386,6 +386,8 @@ tuple_data_split_internal(Oid relid, char *tupdata, (errcode(ERRCODE_DATA_CORRUPTED), errmsg("end of tuple reached without looking at all its data"))); + relation_close(rel, AccessShareLock); + return makeArrayResult(raw_attrs, CurrentMemoryContext); }