Re: Postgres fdw: Bug in InitPgFdwOptions initialization can cause an esoteric crash

From: Lucas Jeffrey <luquijeffrey(at)gmail(dot)com>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org, etsuro(dot)fujita(at)gmail(dot)com, masao(dot)fujii(at)gmail(dot)com, Amit Langote <amitlangote09(at)gmail(dot)com>
Subject: Re: Postgres fdw: Bug in InitPgFdwOptions initialization can cause an esoteric crash
Date: 2026-11-08 21:50:55
Message-ID: CAObkL7NxbVG3ioG9ZDii5aKt0Cw+SJYFfAnJvzMBFSvm+PHO1Q@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi hackers,
I created a v0 of a patch to solve this issue

On Sat, Nov 7, 2026 at 3:11 PM Lucas Jeffrey <luquijeffrey(at)gmail(dot)com> wrote:
>
> Hi hackers,
> After doing some research I found a bug in the initialization of the
> postgres_fdw_options static object.
> The bug itself is that the static pointer postgres_fdw_options is
> assigned when the memory is allocated
> instead of getting assigned when the full initialization loop is completed.
>
> This is roughly what the function does (with my comments explaining the bug):
>
> static void
> InitPgFdwOptions(void)
> {
> ...
> //1: checks if already initialized
> /* Prevent redundant initialization. */
> if (postgres_fdw_options)
> return;
>
> ...
> //2: allocates memory for postgres_fdw_options
> postgres_fdw_options = (PgFdwOption *)
> MemoryContextAlloc(TopMemoryContext,
> sizeof(PgFdwOption) * num_libpq_opts +
> sizeof(non_libpq_options));
>
> popt = postgres_fdw_options;
>
> //3: iterates over libpq_options and copies them into postgres_fdw_options:
> for (lopt = libpq_options; lopt->keyword; lopt++)
> {
> ...
> // 3.1: This strdup may throw an OOM error if allocation fails (error
> not well handled)
> popt->keyword = MemoryContextStrdup(TopMemoryContext,
> lopt->keyword);
> ...
> }
>
> ...
> }
>
>
> the problem with this function is:
> if this first allocation succeeds: MemoryContextAlloc
> but this allocation fails: MemoryContextStrdup
> it leaves a partially allocated object which will cause trubbles later.
> The problem exacerbates in the first check (1: checks if already initialized)
> it will not get re-initialized and checks to that pointer will cause troubles.
>
> I instrumented the code to force the behavior and I was able to found a crasher:
> postgres=# BEGIN;
> BEGIN
> postgres=*# CREATE EXTENSION postgres_fdw;
> ERROR: out of memory
> DETAIL: FAULT INJECTION: Simulated out of memory error for testing.
> (this is to test allocation after failing)
> CONTEXT: SQL statement "CREATE FOREIGN DATA WRAPPER postgres_fdw
> HANDLER postgres_fdw_handler
> VALIDATOR postgres_fdw_validator"
> extension script file "postgres_fdw--1.0.sql", near line 16
> postgres=!# END;
> ROLLBACK
> postgres=# BEGIN;
> BEGIN
> postgres=*# CREATE EXTENSION postgres_fdw;
> CREATE EXTENSION
> postgres=*# CREATE SERVER test_server
> FOREIGN DATA WRAPPER postgres_fdw
> OPTIONS (
> host 'localhost',
> dbname 'postgres'
> );
> server closed the connection unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
> The connection to the server was lost. Attempting reset: Failed.
> !?>
>
>
> Program received signal SIGSEGV, Segmentation fault.
> __strcmp_avx2 () at ../sysdeps/x86_64/multiarch/strcmp-avx2.S:994
> warning: 994 ../sysdeps/x86_64/multiarch/strcmp-avx2.S: No such
> file or directory
> (gdb) bt
> #0 __strcmp_avx2 () at ../sysdeps/x86_64/multiarch/strcmp-avx2.S:994
> #1 0x00007735787705ce in is_valid_option (context=<optimized out>,
> keyword=0x57b237f2ffb8 "host") at option.c:412
> #2 postgres_fdw_validator (fcinfo=<optimized out>) at option.c:83
> #3 0x000057b1f9b1c2fe in FunctionCall2Coll (flinfo=0x7ffca4bd0530,
> collation=<optimized out>, arg1=<optimized out>, arg2=<optimized out>)
> at fmgr.c:1163
> #4 0x000057b1f9b1ca4a in OidFunctionCall2Coll
> (functionId=functionId(at)entry=24622, collation=collation(at)entry=0,
> arg1=96422953671832, arg2=arg2(at)entry=1417) at fmgr.c:1429
> #5 0x000057b1f9722d12 in transformGenericOptions
> (catalogId=catalogId(at)entry=1417, oldOptions=oldOptions(at)entry=0,
> options=<optimized out>, fdwvalidator=24622) at foreigncmds.c:202
> #6 0x000057b1f97239b6 in CreateForeignServer
> (stmt=stmt(at)entry=0x57b237e357d8) at foreigncmds.c:1036
> #7 0x000057b1f99ae53d in ProcessUtilitySlow
> (pstate=pstate(at)entry=0x57b237e5e120, pstmt=pstmt(at)entry=0x57b237e358a8,
> queryString=queryString(at)entry=0x57b237e34bd0 "CREATE SERVER
> test_server\nFOREIGN DATA WRAPPER postgres_fdw\nOPTIONS (\n host
> 'localhost',\n dbname 'postgres'\n);",
> context=context(at)entry=PROCESS_UTILITY_TOPLEVEL,
> params=params(at)entry=0x0, queryEnv=queryEnv(at)entry=0x0,
> qc=0x7ffca4bd0c50, dest=<optimized out>) at utility.c:1600
> #8 0x000057b1f99ad6c9 in standard_ProcessUtility
> (pstmt=0x57b237e358a8, queryString=0x57b237e34bd0 "CREATE SERVER
> test_server\nFOREIGN DATA WRAPPER postgres_fdw\nOPTIONS (\n host
> 'localhost',\n dbname 'postgres'\n);",
> readOnlyTree=<optimized out>, context=PROCESS_UTILITY_TOPLEVEL,
> params=0x0, queryEnv=0x0, dest=0x57b237e35c68, qc=0x7ffca4bd0c50) at
> utility.c:1069
> #9 0x000057b1f99abdcc in PortalRunUtility
> (portal=portal(at)entry=0x57b237ebaa90, pstmt=pstmt(at)entry=0x57b237e358a8,
> isTopLevel=isTopLevel(at)entry=true,
> setHoldSnapshot=setHoldSnapshot(at)entry=false,
> dest=dest(at)entry=0x57b237e35c68,
> qc=qc(at)entry=0x7ffca4bd0c50) at pquery.c:1256
> #10 0x000057b1f99abf0d in PortalRunMulti
> (portal=portal(at)entry=0x57b237ebaa90, isTopLevel=isTopLevel(at)entry=true,
> setHoldSnapshot=setHoldSnapshot(at)entry=false,
> dest=dest(at)entry=0x57b237e35c68, altdest=altdest(at)entry=0x57b237e35c68,
> qc=qc(at)entry=0x7ffca4bd0c50) at pquery.c:1414
> #11 0x000057b1f99ac452 in PortalRun
> (portal=portal(at)entry=0x57b237ebaa90,
> count=count(at)entry=9223372036854775807,
> isTopLevel=isTopLevel(at)entry=true, dest=dest(at)entry=0x57b237e35c68,
> altdest=altdest(at)entry=0x57b237e35c68,
> qc=qc(at)entry=0x7ffca4bd0c50) at pquery.c:891
> #12 0x000057b1f99a7ded in exec_simple_query
> (query_string=0x57b237e34bd0 "CREATE SERVER test_server\nFOREIGN DATA
> WRAPPER postgres_fdw\nOPTIONS (\n host 'localhost',\n dbname
> 'postgres'\n);") at postgres.c:1297
> #13 0x000057b1f99a9a96 in PostgresMain (dbname=<optimized out>,
> username=<optimized out>) at postgres.c:4946
> #14 0x000057b1f99a3a53 in BackendMain (startup_data=<optimized out>,
> startup_data_len=<optimized out>) at backend_startup.c:124
> #15 0x000057b1f98e0be1 in postmaster_child_launch
> (child_type=<optimized out>, child_slot=1,
> startup_data=startup_data(at)entry=0x7ffca4bd1120,
> startup_data_len=startup_data_len(at)entry=24,
> client_sock=client_sock(at)entry=0x7ffca4bd1140)
> at launch_backend.c:268
> #16 0x000057b1f98e4892 in BackendStartup (client_sock=0x7ffca4bd1140)
> at postmaster.c:3650
> #17 ServerLoop () at postmaster.c:1727
> #18 0x000057b1f98e648d in PostmasterMain (argc=argc(at)entry=5,
> argv=argv(at)entry=0x57b237e2efb0) at postmaster.c:1414
> #19 0x000057b1f9586c0e in main (argc=5, argv=0x57b237e2efb0) at main.c:227
> (gdb) p postgres_fdw_options[0]
> $1 = {keyword = 0x57b237e77bc0 "service", optcontext = 1417,
> is_libpq_opt = true}
> (gdb) p postgres_fdw_options[1]
> $2 = {keyword = 0x57b237e784c8 "user", optcontext = 1418, is_libpq_opt = true}
> (gdb) p postgres_fdw_options[2]
> $3 = {keyword = 0x58900000589 <error: Cannot access memory at address
> 0x58900000589>, optcontext = 1417, is_libpq_opt = 137}
> (gdb) p postgres_fdw_options[3]
> $4 = {keyword = 0x58900000589 <error: Cannot access memory at address
> 0x58900000589>, optcontext = 1417, is_libpq_opt = 137}
>
>
>
>
> And this is the instrumentation I added to force the crash:
> diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
> index 5b539c4eeef..fa3f340809a 100644
> --- a/contrib/postgres_fdw/option.c
> +++ b/contrib/postgres_fdw/option.c
> @@ -332,7 +332,22 @@ InitPgFdwOptions(void)
> sizeof(PgFdwOption)
> * num_libpq_opts +
> sizeof(non_libpq_options));
>
> + /*
> + * LJEFFREY: Instrumentation to test some crafted garbage
> memory allocation that could cause a crash.
> + * This is to test the error handling of the code that calls
> this function.
> + * NOTE: This happens before initialization of the
> postgres_fdw_options object, so good error handling
> + * shouldn't crash
> + */
> + for (int i=0; i < (sizeof(PgFdwOption) * num_libpq_opts +
> +
> sizeof(non_libpq_options)) / sizeof(Oid); i++)
> + {
> + ((Oid*)postgres_fdw_options)[i] = 1417;
> + }
> +
> popt = postgres_fdw_options;
> +
> + // Instrumentation to induce error in the middle of the loop
> + int alloc_count = 0;
> for (lopt = libpq_options; lopt->keyword; lopt++)
> {
> /* Hide debug options, as well as settings we override
> internally. */
> @@ -348,6 +363,13 @@ InitPgFdwOptions(void)
> if (strncmp(lopt->keyword, "oauth_", strlen("oauth_")) == 0)
> continue;
>
> + // LJEFFREY: Instrumentacion para probar out of memory
> + if (++alloc_count == 3)
> + ereport(ERROR,
> + (errcode(ERRCODE_OUT_OF_MEMORY),
> + errmsg("out of memory"),
> + errdetail("FAULT INJECTION: Simulated
> out of memory error for testing. (this is to test allocation after
> failing)")));
> +
> popt->keyword = MemoryContextStrdup(TopMemoryContext,
>
> lopt->keyword);
>
>
>
> NOTE: this instrumentation is not meant to be something that may
> happen in production environments, or at least the chance of happening
> is extremely small
> I'll send a patch in this week to see if this can be fixed easily.
> Regards,
> Lucas Jeffrey

Attachment Content-Type Size
0001-postgres_fdw-Set-the-value-of-global-variable-postgr.patch text/x-patch 1.5 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Sho Ito 2026-11-08 22:12:39 [PATCH] psql: Fix REPACK index completion after a column list
Previous Message Tomas Vondra 2026-11-08 21:07:19 reconsider_full_join_clause fails to consider if COALESCE() arguments are NULL-preserving