From: Tom Lane <tgl@sss.pgh.pa.us>
To: exclusion@gmail.com
cc: pgsql-bugs@lists.postgresql.org, Alexander Korotkov <aekorotkov@gmail.com>
Subject: Re: BUG #17847: Unaligned memory access in ltree_gist
In-reply-to: <17847-171232970bea406b@postgresql.org>
References: <17847-171232970bea406b@postgresql.org>
Comments: In-reply-to PG Bug reporting form <noreply@postgresql.org>
	message dated "Thu, 16 Mar 2023 18:00:01 -0000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3966265.1678995315.1@sss.pgh.pa.us>
Date: Thu, 16 Mar 2023 15:35:15 -0400
Message-ID: <3966266.1678995315@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3966266.1678995315%40sss.pgh.pa.us>
Precedence: bulk

PG Bug reporting form <noreply@postgresql.org> writes:
> When the following query executed with address sanitizers (and
> -fsanitize=alignment):
> CREATE EXTENSION ltree;
> CREATE TABLE lt (t ltree);
> INSERT INTO lt SELECT format('%s.%s', i / 10, i % 10)::ltree FROM
> generate_series(1, 200) i;
> CREATE INDEX ltidx ON lt USING gist (t gist_ltree_ops(siglen=99));

> An incorrect memory access is detected:
> ltree_gist.c:66:12: runtime error: member access within misaligned address
> 0x62500019bfd3 for type 'varattrib_4b', which requires 4 byte alignment

Yeah.  So if you ask me, the problem here is that the option for
user-selectable siglen was added with no thought for the possibility
that there might be undocumented implementation restrictions on the
value.  The code is assuming that siglen is MAXALIGN'd (or at least
int-aligned, I did not look too closely), and there was nothing wrong
with that assumption before.

What I'm inclined to do about this is add a restriction that the siglen
value be a multiple of MAXALIGN.  It doesn't look like the reloption
mechanism has a way to specify that declaratively, but we could probably
get close enough by just making LTREE_GET_SIGLEN throw an error if it's
wrong.  That's not ideal because you could probably get through making
an empty index without hitting the error, but I don't offhand see a
way to make it better.

If we decide that we don't need to back-patch a fix for this, maybe
we could instead extend the reloption mechanism to allow stronger
checks on supplied values.  That might be tolerable given how few
alignment-picky machines there are these days.

I wonder which other opclasses besides ltree have the same issue.

			regards, tom lane