Commit b66827ca authored by Tom Lane's avatar Tom Lane

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
parent e27453bd
...@@ -298,9 +298,8 @@ tuple_data_split_internal(Oid relid, char *tupdata, ...@@ -298,9 +298,8 @@ tuple_data_split_internal(Oid relid, char *tupdata,
TupleDesc tupdesc; TupleDesc tupdesc;
/* Get tuple descriptor from relation OID */ /* Get tuple descriptor from relation OID */
rel = relation_open(relid, NoLock); rel = relation_open(relid, AccessShareLock);
tupdesc = CreateTupleDescCopyConstr(rel->rd_att); tupdesc = RelationGetDescr(rel);
relation_close(rel, NoLock);
raw_attrs = initArrayResult(BYTEAOID, CurrentMemoryContext, false); raw_attrs = initArrayResult(BYTEAOID, CurrentMemoryContext, false);
nattrs = tupdesc->natts; nattrs = tupdesc->natts;
...@@ -317,7 +316,6 @@ tuple_data_split_internal(Oid relid, char *tupdata, ...@@ -317,7 +316,6 @@ tuple_data_split_internal(Oid relid, char *tupdata,
bytea *attr_data = NULL; bytea *attr_data = NULL;
attr = TupleDescAttr(tupdesc, i); 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 * Tuple header can specify less attributes than tuple descriptor as
...@@ -327,6 +325,8 @@ tuple_data_split_internal(Oid relid, char *tupdata, ...@@ -327,6 +325,8 @@ tuple_data_split_internal(Oid relid, char *tupdata,
*/ */
if (i >= (t_infomask2 & HEAP_NATTS_MASK)) if (i >= (t_infomask2 & HEAP_NATTS_MASK))
is_null = true; is_null = true;
else
is_null = (t_infomask & HEAP_HASNULL) && att_isnull(i, t_bits);
if (!is_null) if (!is_null)
{ {
...@@ -386,6 +386,8 @@ tuple_data_split_internal(Oid relid, char *tupdata, ...@@ -386,6 +386,8 @@ tuple_data_split_internal(Oid relid, char *tupdata,
(errcode(ERRCODE_DATA_CORRUPTED), (errcode(ERRCODE_DATA_CORRUPTED),
errmsg("end of tuple reached without looking at all its data"))); errmsg("end of tuple reached without looking at all its data")));
relation_close(rel, AccessShareLock);
return makeArrayResult(raw_attrs, CurrentMemoryContext); return makeArrayResult(raw_attrs, CurrentMemoryContext);
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment