From 57dca6faa9bdfd3c2ad2805fc088dc768d36fcf0 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Sat, 15 Feb 2025 13:13:19 +1300 Subject: [PATCH] Fix explicit valgrind interaction in read_stream.c. This is a back-patch of commits 2a8a0067 and 2509b857 into REL_17_STABLE. It's doesn't fix any known live bug in PostgreSQL v17 itself, but an extension could in theory have used the per-buffer data feature and seen spurious errors under Valgrind. Discussion: https://postgr.es/m/CA%2BhUKG%2Bg6aXpi2FEHqeLOzE%2BxYw%3DOV%2B-N5jhOEnnV%2BF0USM9xA%40mail.gmail.com --- src/backend/storage/aio/read_stream.c | 38 ++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c index 9b962c301bf..b5be92d4611 100644 --- a/src/backend/storage/aio/read_stream.c +++ b/src/backend/storage/aio/read_stream.c @@ -176,9 +176,20 @@ read_stream_get_block(ReadStream *stream, void *per_buffer_data) if (blocknum != InvalidBlockNumber) stream->buffered_blocknum = InvalidBlockNumber; else + { + /* + * Tell Valgrind that the per-buffer data is undefined. That replaces + * the "noaccess" state that was set when the consumer moved past this + * entry last time around the queue, and should also catch callbacks + * that fail to initialize data that the buffer consumer later + * accesses. On the first go around, it is undefined already. + */ + VALGRIND_MAKE_MEM_UNDEFINED(per_buffer_data, + stream->per_buffer_data_size); blocknum = stream->callback(stream, stream->callback_private_data, per_buffer_data); + } return blocknum; } @@ -687,8 +698,11 @@ read_stream_next_buffer(ReadStream *stream, void **per_buffer_data) } #ifdef CLOBBER_FREED_MEMORY - /* Clobber old buffer and per-buffer data for debugging purposes. */ + /* Clobber old buffer for debugging purposes. */ stream->buffers[oldest_buffer_index] = InvalidBuffer; +#endif + +#if defined(CLOBBER_FREED_MEMORY) || defined(USE_VALGRIND) /* * The caller will get access to the per-buffer data, until the next call. @@ -697,11 +711,23 @@ read_stream_next_buffer(ReadStream *stream, void **per_buffer_data) * that is holding a dangling pointer to it. */ if (stream->per_buffer_data) - wipe_mem(get_per_buffer_data(stream, - oldest_buffer_index == 0 ? - stream->queue_size - 1 : - oldest_buffer_index - 1), - stream->per_buffer_data_size); + { + void *per_buffer_data; + + per_buffer_data = get_per_buffer_data(stream, + oldest_buffer_index == 0 ? + stream->queue_size - 1 : + oldest_buffer_index - 1); + +#if defined(CLOBBER_FREED_MEMORY) + /* This also tells Valgrind the memory is "noaccess". */ + wipe_mem(per_buffer_data, stream->per_buffer_data_size); +#elif defined(USE_VALGRIND) + /* Tell it ourselves. */ + VALGRIND_MAKE_MEM_NOACCESS(per_buffer_data, + stream->per_buffer_data_size); +#endif + } #endif /* Pin transferred to caller. */