Re: BUG #17855: Uninitialised memory used when the name type value processed in binary mode of Memoize

On Tue, 21 Mar 2023 at 10:27, PG Bug reporting form
<noreply@postgresql.org> wrote:
> SET enable_seqscan TO off;
>
> CREATE TABLE t (n name);
> INSERT INTO t VALUES('name'), ('name');
> CREATE INDEX t_idx ON t(n);
> ANALYZE t;
>
> SELECT COUNT(*) FROM t t1 INNER JOIN t t2 ON t1.n >= t2.n;
>
> An incorrect memory access detected:
> ==00:00:00:06.975 2318810== Use of uninitialised value of size 8

This is very strange.  I tracked this down to the fact that the
TupleDesc for the heap and the index don't match.

postgres=# select attrelid::regclass,attname,attlen from pg_attribute
where attnum=1 and attrelid in ('t'::regclass,'t_idx'::regclass);
 attrelid | attname | attlen
----------+---------+--------
 t        | n       |     64
 t_idx    | n       |     -2

This seems to be due to pg_opclass defining that cstring should be the
opckeytype for btree index for the name type:

postgres=# select *,opckeytype::regtype from pg_opclass where
opcintype='name'::regtype and opcdefault and opcmethod=403;
  oid  | opcmethod | opcname  | opcnamespace | opcowner | opcfamily |
opcintype | opcdefault | opckeytype | opckeytype
-------+-----------+----------+--------------+----------+-----------+-----------+------------+------------+------------
 10028 |       403 | name_ops |           11 |       10 |      1994 |
      19 | t          |       2275 | cstring
(1 row)

The problem valgrind finds is due to index_deform_tuple_internal(), if
I put a breakpoint in indextuple.c:536 after the values[attnum] =
fetchatt(thisatt, tp + off); then output the stored Datum in my
debugger, I see:

((char *) values[attnum])[0]
110 'n'
((char *) values[attnum])[1]
97 'a'
((char *) values[attnum])[2]
109 'm'
((char *) values[attnum])[3]
101 'e'
((char *) values[attnum])[4]
0 '\0'
...
((char *) values[attnum])[7]
0 '\0'
((char *) values[attnum])[8]
-51 'Í'

Note the value at element 8 (I'd expect) should be 0.  In
nodeMemoize.c when we call datum_image_hash(), we'll hash the full 64
bytes of the name type and when we access element 8, we read
uninitialised memory.

The reason this only seems to happen when Memoize is in binary mode is
because we use datum_image_hash() which will hash the full 64 bytes of
the name Datum.  When in logical mode and in hash joins, we'll use
hashname(), which calls strlen() on the input name Datum and only
hashes the bytes as if the name were a cstring:

return hash_any((unsigned char *) key, strlen(key));

I really don't know if I made a wrong assumption about the safety of
using datum_image_hash(). It really surprises me that it's unsafe to
not access the full 64 bytes of a name Datum.

David