public inbox for [email protected]
help / color / mirror / Atom feedFrom: Matheus Alcantara <[email protected]>
To: Tomas Vondra <[email protected]>
To: Glauber Batista <[email protected]>
To: [email protected]
Subject: Re: Autoprewarm workers terminated due to a segmentation fault
Date: Tue, 09 Jun 2026 19:25:55 -0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CAO+_mTQgQyTYwDh=U8iTnsDmOGyWsZJjUV31SmEYwmw6_xY6Bw@mail.gmail.com>
<[email protected]>
<[email protected]>
On Tue Jun 9, 2026 at 6:44 PM -03, Tomas Vondra wrote:
> So how does it get advanced past the prewarm_stop_idx? I've been unable
> to reproduce it locally, maybe it's platform-specific. The original
> report was from ARM, are you on ARM too, Matheus?
>
Yes, I'm also on ARM. I also set pg_prewarm.autoprewarm_interval=10s on
postgresql.conf, not sure if it make more easier to reproduce.
> But AFAIK the code may not account for read stream callback updating the
> pos to prewarm_stop_idx? The callback may end with (p->pos =
> apw_state->prewarm_stop_idx), and that seems to be past the end of the
> array.
>
Yes, this is my understanding.
> That'd mean the proposed check is generally the correct way to fix this.
> TBH it's not clear to me why this needs to set the *next* entry at the
> end of the loop. Well, it does that so that the loop condition can use
> 'blk', but that seems a bit fragile / confusing, and no one noticed the
> issue.
>
> Maybe this would be a better way to write the while loop?
>
> while (i < apw_state->prewarm_stop_idx)
> {
> blk = block_info[i];
>
> if (blk.tablespace != tablespace ||
> blk.filenumber != filenumber)
> break;
>
> ...
> }
>
>
Is attached patch what are you sugesting? If yes, I agree that looks
better, it's more safe and easier to understand.
--
Matheus Alcantara
EDB: https://www.enterprisedb.com
From ebf5296026b6f07fb30821ee0603fff24817001b Mon Sep 17 00:00:00 2001
From: Matheus Alcantara <[email protected]>
Date: Tue, 9 Jun 2026 17:59:20 -0300
Subject: [PATCH v2] Fix out-of-bounds access in autoprewarm worker
The read stream callback apw_read_stream_next_block() advances p->pos
through the block_info array. When processing the last block, it
increments p->pos to prewarm_stop_idx before returning. The callback
itself is safe because it checks bounds before accessing the array.
However, the caller assigned blk from block_info[i] at the end of the
loop body, before the loop condition was re-evaluated. When i equaled
prewarm_stop_idx, this accessed memory beyond the allocated DSM segment,
causing a segfault.
Restructure the loop to check bounds at the top and assign blk at the
beginning of the loop body, where it is always safe. This avoids the
need for an explicit bounds check at the end.
Author: Matheus Alcantara <[email protected]>
Reported-by: Glauber Batista <[email protected]>
Discussion: https://www.postgresql.org/message-id/CAO%2B_mTQgQyTYwDh%3DU8iTnsDmOGyWsZJjUV31SmEYwmw6_xY6Bw%40mail...
---
contrib/pg_prewarm/autoprewarm.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c
index ba0bc8e6d4a..f3569b12d11 100644
--- a/contrib/pg_prewarm/autoprewarm.c
+++ b/contrib/pg_prewarm/autoprewarm.c
@@ -572,16 +572,23 @@ autoprewarm_database_main(Datum main_arg)
* valid forks or run out of options, we'll close the relation and
* move on.
*/
- while (i < apw_state->prewarm_stop_idx &&
- blk.tablespace == tablespace &&
- blk.filenumber == filenumber)
+ while (i < apw_state->prewarm_stop_idx)
{
- ForkNumber forknum = blk.forknum;
+ ForkNumber forknum;
BlockNumber nblocks;
struct AutoPrewarmReadStreamData p;
ReadStream *stream;
Buffer buf;
+ blk = block_info[i];
+
+ /* Stop when we reach a different relation. */
+ if (blk.tablespace != tablespace ||
+ blk.filenumber != filenumber)
+ break;
+
+ forknum = blk.forknum;
+
/*
* smgrexists is not safe for illegal forknum, hence check whether
* the passed forknum is valid before using it in smgrexists.
@@ -645,7 +652,6 @@ autoprewarm_database_main(Datum main_arg)
/* Advance i past all the blocks just prewarmed. */
i = p.pos;
- blk = block_info[i];
}
relation_close(rel, AccessShareLock);
--
2.50.1 (Apple Git-155)
Attachments:
[text/plain] v2-0001-Fix-out-of-bounds-access-in-autoprewarm-worker.patch (2.5K, ../[email protected]/2-v2-0001-Fix-out-of-bounds-access-in-autoprewarm-worker.patch)
download | inline diff:
From ebf5296026b6f07fb30821ee0603fff24817001b Mon Sep 17 00:00:00 2001
From: Matheus Alcantara <[email protected]>
Date: Tue, 9 Jun 2026 17:59:20 -0300
Subject: [PATCH v2] Fix out-of-bounds access in autoprewarm worker
The read stream callback apw_read_stream_next_block() advances p->pos
through the block_info array. When processing the last block, it
increments p->pos to prewarm_stop_idx before returning. The callback
itself is safe because it checks bounds before accessing the array.
However, the caller assigned blk from block_info[i] at the end of the
loop body, before the loop condition was re-evaluated. When i equaled
prewarm_stop_idx, this accessed memory beyond the allocated DSM segment,
causing a segfault.
Restructure the loop to check bounds at the top and assign blk at the
beginning of the loop body, where it is always safe. This avoids the
need for an explicit bounds check at the end.
Author: Matheus Alcantara <[email protected]>
Reported-by: Glauber Batista <[email protected]>
Discussion: https://www.postgresql.org/message-id/CAO%2B_mTQgQyTYwDh%3DU8iTnsDmOGyWsZJjUV31SmEYwmw6_xY6Bw%40mail.gmail.com
---
contrib/pg_prewarm/autoprewarm.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c
index ba0bc8e6d4a..f3569b12d11 100644
--- a/contrib/pg_prewarm/autoprewarm.c
+++ b/contrib/pg_prewarm/autoprewarm.c
@@ -572,16 +572,23 @@ autoprewarm_database_main(Datum main_arg)
* valid forks or run out of options, we'll close the relation and
* move on.
*/
- while (i < apw_state->prewarm_stop_idx &&
- blk.tablespace == tablespace &&
- blk.filenumber == filenumber)
+ while (i < apw_state->prewarm_stop_idx)
{
- ForkNumber forknum = blk.forknum;
+ ForkNumber forknum;
BlockNumber nblocks;
struct AutoPrewarmReadStreamData p;
ReadStream *stream;
Buffer buf;
+ blk = block_info[i];
+
+ /* Stop when we reach a different relation. */
+ if (blk.tablespace != tablespace ||
+ blk.filenumber != filenumber)
+ break;
+
+ forknum = blk.forknum;
+
/*
* smgrexists is not safe for illegal forknum, hence check whether
* the passed forknum is valid before using it in smgrexists.
@@ -645,7 +652,6 @@ autoprewarm_database_main(Datum main_arg)
/* Advance i past all the blocks just prewarmed. */
i = p.pos;
- blk = block_info[i];
}
relation_close(rel, AccessShareLock);
--
2.50.1 (Apple Git-155)
view thread (10+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Autoprewarm workers terminated due to a segmentation fault
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox