From: Tom Lane <tgl@sss.pgh.pa.us>
To: anisimow.d@gmail.com
cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17462: Invalid memory access in heapam_tuple_lock
In-reply-to: <17462-9c98a0f00df9bd36@postgresql.org>
References: <17462-9c98a0f00df9bd36@postgresql.org>
Comments: In-reply-to PG Bug reporting form <noreply@postgresql.org>
	message dated "Mon, 11 Apr 2022 07:46:41 -0000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <286747.1649692519.1@sss.pgh.pa.us>
Date: Mon, 11 Apr 2022 11:55:19 -0400
Message-ID: <286748.1649692519@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/286748.1649692519%40sss.pgh.pa.us>
Precedence: bulk

PG Bug reporting form <noreply@postgresql.org> writes:
> When running parallel queries using pgbench with valgrind-enabled server:
> pgbench -i -s 1
> pgbench -t 1000 -c 10 -j 10
> I get:
> ==00:00:03:09.642 456530== Invalid read of size 2

Reproduced here.  It's surprising that nobody noticed this before,
because AFAICS the bug is pretty old: it dates to somebody foolishly
deciding that heap_fetch didn't need its keep_buf argument, which
evidently happened in v12 (didn't track down the exact commit yet).
As you say, valgrind would not have caught this problem before
1e0dfd166, but that's not so new anymore either.

In principle, this is showing an actual bug, because once we drop
the buffer pin somebody could replace the page before we get done
examining the tuple.  I'm not sure what the odds are of that happening
in the field, but they're probably mighty low because a just-accessed
buffer should not be high priority for replacement.

My inclination for a fix is to revert the removal of the keep_buf argument
and go back to having heapam_tuple_lock and other callers release the
buffer after they are done.  However, that's problematic in released
branches, because it seems likely that there are outside callers of
heap_fetch.  Can we get away with only fixing this in HEAD?

			regards, tom lane