pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedFrom: Michael Paquier <michael@paquier.xyz>
To: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Cc: ilia.kashintsev@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #19612: SEGV in ParseConfigFp() in guc-file.l
Date: Sun, 16 Aug 2026 15:05:13 +0900
Message-ID: <aoFTGfKWOFxVZojJ@paquier.xyz> (raw)
In-Reply-To: <CAB8bMitdZN9RX+Yf8j1A2sBsrATNN39FX7biUi1T4G7V2XLHag@mail.gmail.com>
References: <19612-24ccb4fc6da7786f@postgresql.org>
<CAB8bMivnXcb7RRd=EKdY_nBOqR=6iJPwZx1oEGz+ayfqzj9HVQ@mail.gmail.com>
<an1__bbJ4MNfy9Lp@paquier.xyz>
<CAB8bMitdZN9RX+Yf8j1A2sBsrATNN39FX7biUi1T4G7V2XLHag@mail.gmail.com>
On Thu, Aug 13, 2026 at 02:00:58PM +0500, Andrey Rachitskiy wrote:
> Thanks for review, and for catching the
> yylex_init() warning. Sorry, I missed that one.
Please do not top-post. Please see:
https://en.wikipedia.org/wiki/Posting_style#Bottom-posting
> The non-volatile copy for yylex_init() looks right. That call writes
> through a yyscan_t *, so &scanner after the volatile change is exactly
> the qualifier discard the compiler reports. Casting the address would
> only silence the warning. scanner_init is never read after the
> longjmp, so it does not need to be volatile.
>
> I would drop the if (scanner) guard in cleanup.
I don't follow this argument. ParseConfigFp(), ParseConfigFile() or
ProcessConfigFile() can be called with an elevel lower than ERROR, and
we have quite a few callers that do so.
It seems to me that we should also have a `goto cleanup` if
yylex_init() fails, also pointing at d663f150b5ed that has switched
the scanner to be reentrant where yylex_init() has been added.
I have been on the edge about backpatching that, but as that's only
v18, perhaps that's OK. It does not change the fact that the error
reported is still confusing if one has the idea to use such a
configuration layer, but I cannot really get convinced that this is
worth tweaking: nobody is going to do that, so I don't really feel bad
about letting flex complain as long as we handle the states accessed
in the sigjumps in a better way.
Thoughts or comments are welcome.
--
Michael
From aa0bcbbbbe164671a73c9f0702c0fbcadb4a8da4 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Sun, 16 Aug 2026 14:49:16 +0900
Subject: [PATCH v2] Fix ASAN failure after flex errors in GUC file parsing
As detected by ASAN, the scanner value used when parsing GUC files can
be indeterminate when the flex error handler sigjumps to old cleanup
path, before yylex_init() is called.
The flex scanner state is now made volatile in ParseConfigFp(), since
its value is assigned after sigsetjmp() and cna be accessed after
siglongjmp(). yylex_init() cannot use a volatile pointer; a temporary
variable is used before assigning the result of yylex_init() to it.
Oversight in d663f150b5ed.
Reported-by: Ilia Kashintsev <ilia.kashintsev@gmail.com>
Discussion: https://postgr.es/m/19612-24ccb4fc6da7786f@postgresql.org
Backpatch-through: 18
---
src/backend/utils/misc/guc-file.l | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index 58669a67e050..84c102717eeb 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -354,7 +354,8 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel,
unsigned int save_ConfigFileLineno = ConfigFileLineno;
sigjmp_buf *save_GUC_flex_fatal_jmp = GUC_flex_fatal_jmp;
sigjmp_buf flex_fatal_jmp;
- yyscan_t scanner;
+ volatile yyscan_t scanner = NULL;
+ yyscan_t scanner_init = NULL; /* non-volatile for yylex_init() */
struct yyguts_t *yyg; /* needed for yytext macro */
volatile YY_BUFFER_STATE lex_buffer = NULL;
int errorcount;
@@ -384,8 +385,12 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel,
ConfigFileLineno = 1;
errorcount = 0;
- if (yylex_init(&scanner) != 0)
+ if (yylex_init(&scanner_init) != 0)
+ {
elog(elevel, "yylex_init() failed: %m");
+ goto cleanup;
+ }
+ scanner = scanner_init;
yyg = (struct yyguts_t *) scanner;
lex_buffer = yy_create_buffer(fp, YY_BUF_SIZE, scanner);
@@ -559,8 +564,11 @@ parse_error:
}
cleanup:
- yy_delete_buffer(lex_buffer, scanner);
- yylex_destroy(scanner);
+ if (scanner)
+ {
+ yy_delete_buffer(lex_buffer, scanner);
+ yylex_destroy(scanner);
+ }
/* Each recursion level must save and restore these static variables. */
ConfigFileLineno = save_ConfigFileLineno;
GUC_flex_fatal_jmp = save_GUC_flex_fatal_jmp;
--
2.55.0
Attachments:
[text/plain] v2-0001-Fix-ASAN-failure-after-flex-errors-in-GUC-file-pa.patch (2.4K, ../aoFTGfKWOFxVZojJ@paquier.xyz/2-v2-0001-Fix-ASAN-failure-after-flex-errors-in-GUC-file-pa.patch)
download | inline diff:
From aa0bcbbbbe164671a73c9f0702c0fbcadb4a8da4 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Sun, 16 Aug 2026 14:49:16 +0900
Subject: [PATCH v2] Fix ASAN failure after flex errors in GUC file parsing
As detected by ASAN, the scanner value used when parsing GUC files can
be indeterminate when the flex error handler sigjumps to old cleanup
path, before yylex_init() is called.
The flex scanner state is now made volatile in ParseConfigFp(), since
its value is assigned after sigsetjmp() and cna be accessed after
siglongjmp(). yylex_init() cannot use a volatile pointer; a temporary
variable is used before assigning the result of yylex_init() to it.
Oversight in d663f150b5ed.
Reported-by: Ilia Kashintsev <ilia.kashintsev@gmail.com>
Discussion: https://postgr.es/m/19612-24ccb4fc6da7786f@postgresql.org
Backpatch-through: 18
---
src/backend/utils/misc/guc-file.l | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index 58669a67e050..84c102717eeb 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -354,7 +354,8 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel,
unsigned int save_ConfigFileLineno = ConfigFileLineno;
sigjmp_buf *save_GUC_flex_fatal_jmp = GUC_flex_fatal_jmp;
sigjmp_buf flex_fatal_jmp;
- yyscan_t scanner;
+ volatile yyscan_t scanner = NULL;
+ yyscan_t scanner_init = NULL; /* non-volatile for yylex_init() */
struct yyguts_t *yyg; /* needed for yytext macro */
volatile YY_BUFFER_STATE lex_buffer = NULL;
int errorcount;
@@ -384,8 +385,12 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel,
ConfigFileLineno = 1;
errorcount = 0;
- if (yylex_init(&scanner) != 0)
+ if (yylex_init(&scanner_init) != 0)
+ {
elog(elevel, "yylex_init() failed: %m");
+ goto cleanup;
+ }
+ scanner = scanner_init;
yyg = (struct yyguts_t *) scanner;
lex_buffer = yy_create_buffer(fp, YY_BUF_SIZE, scanner);
@@ -559,8 +564,11 @@ parse_error:
}
cleanup:
- yy_delete_buffer(lex_buffer, scanner);
- yylex_destroy(scanner);
+ if (scanner)
+ {
+ yy_delete_buffer(lex_buffer, scanner);
+ yylex_destroy(scanner);
+ }
/* Each recursion level must save and restore these static variables. */
ConfigFileLineno = save_ConfigFileLineno;
GUC_flex_fatal_jmp = save_GUC_flex_fatal_jmp;
--
2.55.0
[application/pgp-signature] signature.asc (832B, ../aoFTGfKWOFxVZojJ@paquier.xyz/3-signature.asc)
download
view thread (8+ messages) latest in thread
Message-ID: <aoFTGfKWOFxVZojJ@paquier.xyz>
Permalink: ../aoFTGfKWOFxVZojJ@paquier.xyz/
Also on: postgresql.org/message-id/aoFTGfKWOFxVZojJ@paquier.xyz
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: pgsql-bugs@postgresql.org
Cc: michael@paquier.xyz, pl0h0yp1@gmail.com, ilia.kashintsev@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #19612: SEGV in ParseConfigFp() in guc-file.l
In-Reply-To: <aoFTGfKWOFxVZojJ@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox