agora inbox for pgsql-bugs@postgresql.org  
help / color / mirror / Atom feed
BUG #19612: SEGV in ParseConfigFp() in guc-file.l
8+ messages / 4 participants
[nested] [flat]

* BUG #19612: SEGV in ParseConfigFp() in guc-file.l
@ 2026-08-07 13:32  PG Bug reporting form <noreply@postgresql.org>
  0 siblings, 1 reply; 8+ messages in thread

From: PG Bug reporting form @ 2026-08-07 13:32 UTC (permalink / raw)
  To: pgsql-bugs@lists.postgresql.org; +Cc: ilia.kashintsev@gmail.com

The following bug has been logged on the website:

Bug reference:      19612
Logged by:          Ilia Kashintsev
Email address:      ilia.kashintsev@gmail.com
PostgreSQL version: 19beta2
Operating system:   Ubuntu 24.04.4 LTS
Description:        

Hello maintainers!
I have found a SEGV on unknown address in ParseConfigFp().

The error itself is caused by passing a directory to the "include"
statement in the configuration file. The current checks do not account
for such error, so the first reading attempt occurring via
"while ((token = yylex(scanner)))" -> yy_get_next_buffer -> YY_INPUT
results in a fatal Flex error. After that execution goes to the
cleanup, and the state is not "sane enough for yy_delete_buffer()",
resulting in a crash on dereferences in YY_CURRENT_BUFFER.

Steps to reproduce:

1) Build the project with ASAN;
sudo mkdir -p /builds2
sudo chown "$(whoami)" /builds2

mkdir -p asan_build
cd asan_build
export CC=clang
export CXX=clang++
export CFLAGS="-O1 -g -fsanitize=address -fno-omit-frame-pointer"
export CXXFLAGS="-O1 -g -fsanitize=address -fno-omit-frame-pointer"
export LDFLAGS="-fsanitize=address"

../postgres/configure --prefix=/builds2/pg-asan
make -j
sudo make install

2) Run with the example config:

echo "include 'directory'" > error.conf
mkdir directory
/builds2/pg-asan/bin/postgres -c config_file=./error.conf

Sanitizer output:
2026-08-06 11:47:13.488 GMT [219826] LOG:  input in flex scanner failed at
file "/home/reproduce/asan_build/directory" line 1
AddressSanitizer:DEADLYSIGNAL
=================================================================
==219826==ERROR: AddressSanitizer: SEGV on unknown address (pc
0x62a2e32967a3 bp 0x7ffc3751ed70 sp 0x7ffc3751eb20 T0)
==219826==The signal is caused by a READ memory access.
==219826==Hint: this fault was caused by a dereference of a high value
address (see register values below).  Disassemble the provided pc to learn
which register was used.
    #0 0x62a2e32967a3 in GUC_yy_delete_buffer
/home/reproduce/asan_build/src/backend/utils/misc/guc-file.c:1631:12
    #1 0x62a2e32967a3 in ParseConfigFp
/home/reproduce/asan_build/../postgres/src/backend/utils/misc/guc-file.l:562:2
    #2 0x62a2e3294d6e in ParseConfigFile
/home/reproduce/asan_build/../postgres/src/backend/utils/misc/guc-file.l:263:7
    #3 0x62a2e3296463 in ParseConfigFp
/home/reproduce/asan_build/../postgres/src/backend/utils/misc/guc-file.l:473:9
    #4 0x62a2e3294d6e in ParseConfigFile
/home/reproduce/asan_build/../postgres/src/backend/utils/misc/guc-file.l:263:7
    #5 0x62a2e3277058 in ProcessConfigFileInternal
/home/reproduce/asan_build/../postgres/src/backend/utils/misc/guc.c:299:7
    #6 0x62a2e3294b16 in ProcessConfigFile
/home/reproduce/asan_build/../postgres/src/backend/utils/misc/guc-file.l:153:9
    #7 0x62a2e327b0f0 in SelectConfigFiles
/home/reproduce/asan_build/../postgres/src/backend/utils/misc/guc.c:1733:2
    #8 0x62a2e2ccebcc in PostmasterMain
/home/reproduce/asan_build/../postgres/src/backend/postmaster/postmaster.c:790:7
    #9 0x62a2e2a01831 in main
/home/reproduce/asan_build/../postgres/src/backend/main/main.c:231:4
    #10 0x73e7983d71c9 in __libc_start_call_main
csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #11 0x73e7983d728a in __libc_start_main csu/../csu/libc-start.c:360:3
    #12 0x62a2e22c9ef4 in _start (/builds2/pg-asan/bin/postgres+0x381ef4)
(BuildId: 8022979c2ccf668e43e16c68d221ad47bf314c32)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV
/home/reproduce/asan_build/src/backend/utils/misc/guc-file.c:1631:12 in
GUC_yy_delete_buffer
==219826==ABORTING

Suggested fix:
Probably could be done more elegantly,
but a check for a directory resolves the issue:

diff --git a/src/backend/utils/misc/guc-file.l
b/src/backend/utils/misc/guc-file.l
index 58669a6..e6c810a 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -10,6 +10,7 @@
 #include "postgres.h"

 #include <ctype.h>
+#include <sys/stat.h>
 #include <unistd.h>

 #include "common/file_utils.h"
@@ -237,6 +238,18 @@ ParseConfigFile(const char *config_file, bool strict,
        }

        fp = AllocateFile(abs_path, "r");
+       if (fp)
+       {
+               struct stat st;
+
+               if (fstat(fileno(fp), &st) == 0 && S_ISDIR(st.st_mode))
+               {
+                       FreeFile(fp);
+                       fp = NULL;
+                       errno = EISDIR;
+               }
+       }
+
        if (!fp)
        {
                if (strict)








^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #19612: SEGV in ParseConfigFp() in guc-file.l
@ 2026-08-11 06:30  Andrey Rachitskiy <pl0h0yp1@gmail.com>
  parent: PG Bug reporting form <noreply@postgresql.org>
  0 siblings, 1 reply; 8+ messages in thread

From: Andrey Rachitskiy @ 2026-08-11 06:30 UTC (permalink / raw)
  To: ilia.kashintsev@gmail.com; pgsql-bugs@lists.postgresql.org

Hi, Ilia!

Thanks for the report.

fopen() of a directory can succeed.  Flex then fails with "input in
flex scanner failed".  GUC_flex_fatal() longjmps to ParseConfigFp()
cleanup, which calls yy_delete_buffer()/yylex_destroy() and crashes.

Commit 4b496a3583e already marked the YY_BUFFER_STATE local volatile
for that longjmp path.  Commit d663f150b5e made the scanner reentrant
and added a yyscan_t local used in the same cleanup.  That local is
written after sigsetjmp and read after siglongjmp, but was not
volatile, so its value is indeterminate after the jump.

вт, 11 авг. 2026 г. в 09:55, PG Bug reporting form <noreply@postgresql.org>:

> The following bug has been logged on the website:
>
> Bug reference:      19612
> Logged by:          Ilia Kashintsev
> Email address:      ilia.kashintsev@gmail.com
> PostgreSQL version: 19beta2
> Operating system:   Ubuntu 24.04.4 LTS
> Description:
>
> Hello maintainers!
> I have found a SEGV on unknown address in ParseConfigFp().
>
> The error itself is caused by passing a directory to the "include"
> statement in the configuration file. The current checks do not account
> for such error, so the first reading attempt occurring via
> "while ((token = yylex(scanner)))" -> yy_get_next_buffer -> YY_INPUT
> results in a fatal Flex error. After that execution goes to the
> cleanup, and the state is not "sane enough for yy_delete_buffer()",
> resulting in a crash on dereferences in YY_CURRENT_BUFFER.
>
> Steps to reproduce:
>
> 1) Build the project with ASAN;
> sudo mkdir -p /builds2
> sudo chown "$(whoami)" /builds2
>
> mkdir -p asan_build
> cd asan_build
> export CC=clang
> export CXX=clang++
> export CFLAGS="-O1 -g -fsanitize=address -fno-omit-frame-pointer"
> export CXXFLAGS="-O1 -g -fsanitize=address -fno-omit-frame-pointer"
> export LDFLAGS="-fsanitize=address"
>
> ../postgres/configure --prefix=/builds2/pg-asan
> make -j
> sudo make install
>
> 2) Run with the example config:
>
> echo "include 'directory'" > error.conf
> mkdir directory
> /builds2/pg-asan/bin/postgres -c config_file=./error.conf
>
> Sanitizer output:
> 2026-08-06 11:47:13.488 GMT [219826] LOG:  input in flex scanner failed at
> file "/home/reproduce/asan_build/directory" line 1
> AddressSanitizer:DEADLYSIGNAL
> =================================================================
> ==219826==ERROR: AddressSanitizer: SEGV on unknown address (pc
> 0x62a2e32967a3 bp 0x7ffc3751ed70 sp 0x7ffc3751eb20 T0)
> ==219826==The signal is caused by a READ memory access.
> ==219826==Hint: this fault was caused by a dereference of a high value
> address (see register values below).  Disassemble the provided pc to learn
> which register was used.
>     #0 0x62a2e32967a3 in GUC_yy_delete_buffer
> /home/reproduce/asan_build/src/backend/utils/misc/guc-file.c:1631:12
>     #1 0x62a2e32967a3 in ParseConfigFp
>
> /home/reproduce/asan_build/../postgres/src/backend/utils/misc/guc-file.l:562:2
>     #2 0x62a2e3294d6e in ParseConfigFile
>
> /home/reproduce/asan_build/../postgres/src/backend/utils/misc/guc-file.l:263:7
>     #3 0x62a2e3296463 in ParseConfigFp
>
> /home/reproduce/asan_build/../postgres/src/backend/utils/misc/guc-file.l:473:9
>     #4 0x62a2e3294d6e in ParseConfigFile
>
> /home/reproduce/asan_build/../postgres/src/backend/utils/misc/guc-file.l:263:7
>     #5 0x62a2e3277058 in ProcessConfigFileInternal
> /home/reproduce/asan_build/../postgres/src/backend/utils/misc/guc.c:299:7
>     #6 0x62a2e3294b16 in ProcessConfigFile
>
> /home/reproduce/asan_build/../postgres/src/backend/utils/misc/guc-file.l:153:9
>     #7 0x62a2e327b0f0 in SelectConfigFiles
> /home/reproduce/asan_build/../postgres/src/backend/utils/misc/guc.c:1733:2
>     #8 0x62a2e2ccebcc in PostmasterMain
>
> /home/reproduce/asan_build/../postgres/src/backend/postmaster/postmaster.c:790:7
>     #9 0x62a2e2a01831 in main
> /home/reproduce/asan_build/../postgres/src/backend/main/main.c:231:4
>     #10 0x73e7983d71c9 in __libc_start_call_main
> csu/../sysdeps/nptl/libc_start_call_main.h:58:16
>     #11 0x73e7983d728a in __libc_start_main csu/../csu/libc-start.c:360:3
>     #12 0x62a2e22c9ef4 in _start (/builds2/pg-asan/bin/postgres+0x381ef4)
> (BuildId: 8022979c2ccf668e43e16c68d221ad47bf314c32)
>
> AddressSanitizer can not provide additional info.
> SUMMARY: AddressSanitizer: SEGV
> /home/reproduce/asan_build/src/backend/utils/misc/guc-file.c:1631:12 in
> GUC_yy_delete_buffer
> ==219826==ABORTING
>
> Suggested fix:
> Probably could be done more elegantly,
> but a check for a directory resolves the issue:
>
> diff --git a/src/backend/utils/misc/guc-file.l
> b/src/backend/utils/misc/guc-file.l
> index 58669a6..e6c810a 100644
> --- a/src/backend/utils/misc/guc-file.l
> +++ b/src/backend/utils/misc/guc-file.l
> @@ -10,6 +10,7 @@
>  #include "postgres.h"
>
>  #include <ctype.h>
> +#include <sys/stat.h>
>  #include <unistd.h>
>
>  #include "common/file_utils.h"
> @@ -237,6 +238,18 @@ ParseConfigFile(const char *config_file, bool strict,
>         }
>
>         fp = AllocateFile(abs_path, "r");
> +       if (fp)
> +       {
> +               struct stat st;
> +
> +               if (fstat(fileno(fp), &st) == 0 && S_ISDIR(st.st_mode))
> +               {
> +                       FreeFile(fp);
> +                       fp = NULL;
> +                       errno = EISDIR;
> +               }
> +       }
> +
>         if (!fp)
>         {
>                 if (strict)
>
>
>
>
>

-- 
Regards,
Rachitskiy Andrey

Attachments:

  [text/x-patch] 0001-Fix-SEGV-after-flex-fatal-in-ParseConfigFp.patch (1.5K, ../../CAB8bMivnXcb7RRd=EKdY_nBOqR=6iJPwZx1oEGz+ayfqzj9HVQ@mail.gmail.com/3-0001-Fix-SEGV-after-flex-fatal-in-ParseConfigFp.patch)
  download | inline diff:
From: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Date: Tue, 11 Aug 2026 11:11:30 +0500
Subject: [PATCH] Fix SEGV in ParseConfigFp after a flex fatal error

ParseConfigFp() longjmps out of fatal flex errors, then cleans up with
yy_delete_buffer()/yylex_destroy().  After the reentrant scanner
conversion, the yyscan_t local is written after sigsetjmp and read
after siglongjmp without being volatile.  That leaves a clobbered
scanner pointer and can SEGV in cleanup (seen with AddressSanitizer
when "include" points at a directory).

Bug: #19612
Reported-by: Ilia Kashintsev <ilia.kashintsev@gmail.com>
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
---
 src/backend/utils/misc/guc-file.l | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index 58669a67e05..a8530a1e2c7 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -354,9 +354,9 @@ 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;	/* must survive siglongjmp */
 	struct yyguts_t *yyg;		/* needed for yytext macro */
-	volatile YY_BUFFER_STATE lex_buffer = NULL;
+	volatile YY_BUFFER_STATE lex_buffer = NULL;	/* must survive siglongjmp */
 	int			errorcount;
 	int			token;
 
-- 
2.43.0


^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #19612: SEGV in ParseConfigFp() in guc-file.l
@ 2026-08-13 08:27  Michael Paquier <michael@paquier.xyz>
  parent: Andrey Rachitskiy <pl0h0yp1@gmail.com>
  0 siblings, 1 reply; 8+ messages in thread

From: Michael Paquier @ 2026-08-13 08:27 UTC (permalink / raw)
  To: Andrey Rachitskiy <pl0h0yp1@gmail.com>; +Cc: ilia.kashintsev@gmail.com; pgsql-bugs@lists.postgresql.org

On Tue, Aug 11, 2026 at 11:30:03AM +0500, Andrey Rachitskiy wrote:
> Commit 4b496a3583e already marked the YY_BUFFER_STATE local volatile
> for that longjmp path.  Commit d663f150b5e made the scanner reentrant
> and added a yyscan_t local used in the same cleanup.  That local is
> written after sigsetjmp and read after siglongjmp, but was not
> volatile, so its value is indeterminate after the jump.

Asan failure reproduced, thanks.  Your patch has missed the following
piece with yylex_init():
guc-file.l:387:17: warning: passing 'volatile yyscan_t *' (aka 'void
*volatile *') to parameter of type 'yyscan_t *' (aka 'void **')
discards qualifiers
[-Wincompatible-pointer-types-discards-qualifiers]   387 |         if
(yylex_init(&scanner) != 0)

I am wondering whether we should just use a non-volatile copy of
"scanner", just for the sake of yylex_init().  The attached seems to
work fine here with asan.

Thoughts?
--
Michael
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index 58669a67e050..13cdba6cd7b6 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;	/* 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,9 @@ 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");
+	scanner = scanner_init;
 	yyg = (struct yyguts_t *) scanner;
 
 	lex_buffer = yy_create_buffer(fp, YY_BUF_SIZE, scanner);
@@ -559,8 +561,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;

Attachments:

  [text/plain] asan-guc-file-l.patch (1.4K, ../../an1__bbJ4MNfy9Lp@paquier.xyz/2-asan-guc-file-l.patch)
  download | inline diff:
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index 58669a67e050..13cdba6cd7b6 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;	/* 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,9 @@ 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");
+	scanner = scanner_init;
 	yyg = (struct yyguts_t *) scanner;
 
 	lex_buffer = yy_create_buffer(fp, YY_BUF_SIZE, scanner);
@@ -559,8 +561,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;

  [application/pgp-signature] signature.asc (832B, ../../an1__bbJ4MNfy9Lp@paquier.xyz/3-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #19612: SEGV in ParseConfigFp() in guc-file.l
@ 2026-08-13 09:00  Andrey Rachitskiy <pl0h0yp1@gmail.com>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 8+ messages in thread

From: Andrey Rachitskiy @ 2026-08-13 09:00 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: ilia.kashintsev@gmail.com; pgsql-bugs@lists.postgresql.org

Hi, Michael!

Thanks for review, and for catching the
yylex_init() warning.  Sorry, I missed that one.

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.

A flex fatal only comes from GUC_flex_fatal() during yylex(), after
yylex_init() has already stored a live scanner.  yylex_init() itself
only allocates the scanner struct.  It does not take the fatal path.
Nothing between sigsetjmp() and "scanner = scanner_init" can call
GUC_flex_fatal() either.  So on the longjmp path, cleanup always sees
a real scanner.
yy_delete_buffer() already returns immediately when lex_buffer is NULL,
and lex_buffer is already volatile and initialized.  yylex_destroy(NULL)
is not a no-op in flex: it dereferences the scanner.  A NULL check would
matter if that case were reachable here.  It is not.

Let's take your revised version without the cleanup change.

чт, 13 авг. 2026 г. в 13:27, Michael Paquier <michael@paquier.xyz>:

> On Tue, Aug 11, 2026 at 11:30:03AM +0500, Andrey Rachitskiy wrote:
> > Commit 4b496a3583e already marked the YY_BUFFER_STATE local volatile
> > for that longjmp path.  Commit d663f150b5e made the scanner reentrant
> > and added a yyscan_t local used in the same cleanup.  That local is
> > written after sigsetjmp and read after siglongjmp, but was not
> > volatile, so its value is indeterminate after the jump.
>
> Asan failure reproduced, thanks.  Your patch has missed the following
> piece with yylex_init():
> guc-file.l:387:17: warning: passing 'volatile yyscan_t *' (aka 'void
> *volatile *') to parameter of type 'yyscan_t *' (aka 'void **')
> discards qualifiers
> [-Wincompatible-pointer-types-discards-qualifiers]   387 |         if
> (yylex_init(&scanner) != 0)
>
> I am wondering whether we should just use a non-volatile copy of
> "scanner", just for the sake of yylex_init().  The attached seems to
> work fine here with asan.
>
> Thoughts?
> --
> Michael
>


-- 
Regards,
Rachitskiy Andrey

^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #19612: SEGV in ParseConfigFp() in guc-file.l
@ 2026-08-16 06:05  Michael Paquier <michael@paquier.xyz>
  parent: Andrey Rachitskiy <pl0h0yp1@gmail.com>
  0 siblings, 2 replies; 8+ messages in thread

From: Michael Paquier @ 2026-08-16 06:05 UTC (permalink / raw)
  To: Andrey Rachitskiy <pl0h0yp1@gmail.com>; +Cc: ilia.kashintsev@gmail.com; pgsql-bugs@lists.postgresql.org

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

^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #19612: SEGV in ParseConfigFp() in guc-file.l
@ 2026-08-16 06:50  Andrey Rachitskiy <pl0h0yp1@gmail.com>
  parent: Michael Paquier <michael@paquier.xyz>
  1 sibling, 0 replies; 8+ messages in thread

From: Andrey Rachitskiy @ 2026-08-16 06:50 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: ilia.kashintsev@gmail.com; pgsql-bugs@lists.postgresql.org

вс, 16 авг. 2026 г. в 11:05, Michael Paquier <michael@paquier.xyz>:

> 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.
>
> Sorry, that argument was too narrow. I was only looking at the
longjmp from a flex fatal, where the scanner is already live.

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.
>
> Agreed.
One gap in the snippet: OK is still true on that path, so we would
report a successful parse.  That should be:
```
if (yylex_init(&scanner_init) != 0)
{
elog(elevel, "yylex_init() failed: %m");
OK = false;
goto cleanup;
}
```
With that, I am fine taking your version.

-- 
Regards,
Rachitskiy Andrey

^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #19612: SEGV in ParseConfigFp() in guc-file.l
@ 2026-08-16 14:32  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Michael Paquier <michael@paquier.xyz>
  1 sibling, 1 reply; 8+ messages in thread

From: Tom Lane @ 2026-08-16 14:32 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Andrey Rachitskiy <pl0h0yp1@gmail.com>; ilia.kashintsev@gmail.com; pgsql-bugs@lists.postgresql.org

Michael Paquier <michael@paquier.xyz> writes:
> 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.

Yeah.  The other thing not being covered here is the possibility
that yy_create_buffer fails and returns NULL.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #19612: SEGV in ParseConfigFp() in guc-file.l
@ 2026-08-16 22:26  Michael Paquier <michael@paquier.xyz>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 8+ messages in thread

From: Michael Paquier @ 2026-08-16 22:26 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andrey Rachitskiy <pl0h0yp1@gmail.com>; ilia.kashintsev@gmail.com; pgsql-bugs@lists.postgresql.org

On Sun, Aug 16, 2026 at 10:32:14AM -0400, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
> > 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.
> 
> Yeah.  The other thing not being covered here is the possibility
> that yy_create_buffer fails and returns NULL.

Are you worried about possible future changes on the flex side?  If I
read src/c99-flex.skl in the flex repo, yy_create_buffer() always uses
yypanic(), which would trigger the sigjmp.  yylex_init() just sets an
ENOMEM and returns NULL.

Saying that, I can see your point in making our code more defensive,
on the assumption that upstream could change things, or based on the
assumption that we would catch failures if the error handling in
guc-file.l is touched one way or another.
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../aoI5El4tC-uBbU2c@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 8+ messages in thread


end of thread, other threads:[~2026-08-16 22:26 UTC | newest]

Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-08-07 13:32 BUG #19612: SEGV in ParseConfigFp() in guc-file.l PG Bug reporting form <noreply@postgresql.org>
2026-08-11 06:30 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-13 08:27   ` Michael Paquier <michael@paquier.xyz>
2026-08-13 09:00     ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-16 06:05       ` Michael Paquier <michael@paquier.xyz>
2026-08-16 06:50         ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-16 14:32         ` Tom Lane <tgl@sss.pgh.pa.us>
2026-08-16 22:26           ` Michael Paquier <michael@paquier.xyz>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox