Thread

SPI versus read/write expanded datums

Tom Lane <tgl@sss.pgh.pa.us> — 2022-08-10T14:51:00Z

In the report at [1] we learned that the SQL-language function
handler is too cavalier about read/write expanded datums that
it receives as input.  A function that receives such a datum
is entitled to scribble on its value, or even delete it.
If the function turns around and passes the datum on to some
other function, the same applies there.  So in general, it can
only be safe to pass such a datum to *one* subsidiary function.
If you want to use the value more than once, you'd better convert
the pointer to read-only.  fmgr_sql wasn't doing that, leading
to the reported bug.

After fixing that, I wondered if we had the same problem anywhere
else, and it didn't take long to think of such a place: SPI.
If you pass a read/write datum to SPI_execute_plan or one of its
siblings, and the executed query references that datum more than
once, you're potentially in trouble.  Even if it does only
reference it once, you might be surprised that your copy of the
datum got modified.

However, we can't install a 100% fix in SPI itself, because
plpgsql intentionally exploits exactly this behavior to optimize
things like "arr := array_append(arr, val)".  I considered the
idea of adding a 90% fix by making _SPI_convert_params() convert
R/W pointers to R/O.  That would protect places using the old-style
"char *Nulls" APIs, and then we'd deem it the responsibility
of callers using ParamListInfo APIs to protect themselves.
I can't get terribly excited about that though, because it'd
be adding complexity and cycles for a problem that seems entirely
theoretical at this point.  I can't find any SPI callers that
would *actually* be passing a R/W datum to a query that'd be
likely to modify it.  The non-plpgsql PLs are at the most risk
of calling a hazardous query, but they all pass "flat" datums
that are the immediate result of a typinput function or the like.

So my inclination is to do nothing about this now, and maybe
nothing ever.  But I thought it'd be a good idea to memorialize
this issue for the archives.

			regards, tom lane

[1] https://www.postgresql.org/message-id/flat/WScDU5qfoZ7PB2gXwNqwGGgDPmWzz08VdydcPFLhOwUKZcdWbblbo-0Lku-qhuEiZoXJ82jpiQU4hOjOcrevYEDeoAvz6nR0IU4IHhXnaCA%3D%40mackler.email