BUG #19612: SEGV in ParseConfigFp() in guc-file.l

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

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

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

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

вс, 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

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

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