Avoid possible longjmp-induced logic error in PLy_trigger_build_args.
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 1 Apr 2024 19:15:03 +0000 (15:15 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 1 Apr 2024 19:15:03 +0000 (15:15 -0400)
The "pltargs" variable wasn't marked volatile, which makes it unsafe
to change its value within the PG_TRY block.  It looks like the worst
outcome would be to fail to release a refcount on Py_None during an
(improbable) error exit, which would likely go unnoticed in the field.
Still, it's a bug.  A one-liner fix could be to mark pltargs volatile,
but on the whole it seems cleaner to arrange things so that we don't
change its value within PG_TRY.

Per report from Xing Guo.  This has been there for quite awhile,
so back-patch to all supported branches.

Discussion: https://postgr.es/m/CACpMh+DLrk=fDv07MNpBT4J413fDAm+gmMXgi8cjPONE+jvzuw@mail.gmail.com

src/pl/plpython/plpy_exec.c

index e06fde1dd90f08a703de4e54b2101d8108d8da96..3145c696996fa4c4aab35e3d151a3172d3731272 100644 (file)
@@ -689,7 +689,7 @@ PLy_trigger_build_args(FunctionCallInfo fcinfo, PLyProcedure *proc, HeapTuple *r
                           *pltrelid,
                           *plttablename,
                           *plttableschema,
-                          *pltargs = NULL,
+                          *pltargs,
                           *pytnew,
                           *pytold,
                           *pltdata;
@@ -713,6 +713,11 @@ PLy_trigger_build_args(FunctionCallInfo fcinfo, PLyProcedure *proc, HeapTuple *r
                        return NULL;
                }
        }
+       else
+       {
+               Py_INCREF(Py_None);
+               pltargs = Py_None;
+       }
 
        PG_TRY();
        {
@@ -856,7 +861,7 @@ PLy_trigger_build_args(FunctionCallInfo fcinfo, PLyProcedure *proc, HeapTuple *r
                        PyObject   *pltarg;
 
                        /* pltargs should have been allocated before the PG_TRY block. */
-                       Assert(pltargs);
+                       Assert(pltargs && pltargs != Py_None);
 
                        for (i = 0; i < tdata->tg_trigger->tgnargs; i++)
                        {
@@ -870,8 +875,7 @@ PLy_trigger_build_args(FunctionCallInfo fcinfo, PLyProcedure *proc, HeapTuple *r
                }
                else
                {
-                       Py_INCREF(Py_None);
-                       pltargs = Py_None;
+                       Assert(pltargs == Py_None);
                }
                PyDict_SetItemString(pltdata, "args", pltargs);
                Py_DECREF(pltargs);