Re: [PATCHES] PL/Python error checking
Michael Fuhr wrote: On Fri, Sep 23, 2005 at 05:03:02PM -0400, Bruce Momjian wrote: Michael Fuhr wrote: http://archives.postgresql.org/pgsql-patches/2005-06/msg00519.php I have backpatched this to 8.0.X. It did not apply cleanly to 7.4.X so if you would like that version patched please submit a matching patch. Thanks. (I don't trust myself to adjust the patch for 7.4.X.) Here's a patch for 7.4. I had to s/PLy_curr_procedure/PLy_last_procedure/ because 7.4 uses the latter where 8.x uses the former. How far back do you want to backpatch? 7.3? 7.2? The patch is pretty simple so I could make patches for older versions if necessary. Applied to 7.4.X. I that that is as far back as we want to go. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] PL/Python error checking
Michael Fuhr wrote: On Mon, Jul 11, 2005 at 08:13:24PM -0600, Michael Fuhr wrote: On Sun, Jul 10, 2005 at 12:58:24AM -0400, Bruce Momjian wrote: I am unclear about backpatching this. We have to weigh the risks of applying or not applying to 8.0.X. Comments? Since 7.4, PL/Python is only available as an untrusted language, so only a database superuser could create an exploitable function. However, it might be possible for an ordinary user to tickle the bug by calling such a function and passing it certain data, either as an argument or as table data. The code is buggy in any case: PyObject_Str() is documented to return NULL on error, and PyString_AsString() doesn't expect a NULL pointer so it segfaults if passed one. Since the patch simply checks for that condition and raises an error instead of calling a function that will segfault and take down the backend, I can't think of what risk applying the patch would have. The greater risk would seem to be in not applying it. I haven't seen this patch applied to other than HEAD. Since it fixes a segmentation fault, should it be backpatched before the new releases? Here's the original patch submission: http://archives.postgresql.org/pgsql-patches/2005-06/msg00519.php I have backpatched this to 8.0.X. It did not apply cleanly to 7.4.X so if you would like that version patched please submit a matching patch. Thanks. (I don't trust myself to adjust the patch for 7.4.X.) -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] PL/Python error checking
On Fri, Sep 23, 2005 at 05:03:02PM -0400, Bruce Momjian wrote:
Michael Fuhr wrote:
http://archives.postgresql.org/pgsql-patches/2005-06/msg00519.php
I have backpatched this to 8.0.X. It did not apply cleanly to 7.4.X so
if you would like that version patched please submit a matching patch.
Thanks. (I don't trust myself to adjust the patch for 7.4.X.)
Here's a patch for 7.4. I had to s/PLy_curr_procedure/PLy_last_procedure/
because 7.4 uses the latter where 8.x uses the former.
How far back do you want to backpatch? 7.3? 7.2? The patch is
pretty simple so I could make patches for older versions if necessary.
--
Michael Fuhr
Index: src/pl/plpython/plpython.c
===
RCS file: /projects/cvsroot/pgsql/src/pl/plpython/plpython.c,v
retrieving revision 1.41.2.1
diff -c -r1.41.2.1 plpython.c
*** src/pl/plpython/plpython.c 4 Jan 2004 00:14:55 - 1.41.2.1
--- src/pl/plpython/plpython.c 24 Sep 2005 02:47:38 -
***
*** 561,566
--- 561,568
if (plval != Py_None !tupdesc-attrs[atti]-attisdropped)
{
plstr = PyObject_Str(plval);
+ if (!plstr)
+ PLy_elog(ERROR, function \%s\ could not
modify tuple, proc-proname);
src = PyString_AsString(plstr);
modvalues[i] =
FunctionCall3(proc-result.out.r.atts[atti].typfunc,
***
*** 847,852
--- 849,856
{
fcinfo-isnull = false;
plrv_so = PyObject_Str(plrv);
+ if (!plrv_so)
+ PLy_elog(ERROR, function \%s\ could not create
return value, proc-proname);
plrv_sc = PyString_AsString(plrv_so);
rv = FunctionCall3(proc-result.out.d.typfunc,
PointerGetDatum(plrv_sc),
***
*** 2117,2123
char *sv;
PyObject *so = PyObject_Str(list);
!
sv = PyString_AsString(so);
PLy_exception_set(PLy_exc_spi_error,
Expected sequence of %d
arguments, got %d. %s,
--- 2121,2129
char *sv;
PyObject *so = PyObject_Str(list);
! if (!so)
! PLy_elog(ERROR, function \%s\ could not execute
plan,
!
PLy_procedure_name(PLy_last_procedure));
sv = PyString_AsString(so);
PLy_exception_set(PLy_exc_spi_error,
Expected sequence of %d
arguments, got %d. %s,
***
*** 2166,2171
--- 2172,2180
if (elem != Py_None)
{
so = PyObject_Str(elem);
+ if (!so)
+ PLy_elog(ERROR, function \%s\ could
not execute plan,
+
PLy_procedure_name(PLy_last_procedure));
sv = PyString_AsString(so);
/*
***
*** 2693,2699
else
vstr = Unknown;
! estr = PyString_AsString(eob);
xstr = PLy_printf(%s: %s, estr, vstr);
Py_DECREF(eob);
--- 2702,2714
else
vstr = Unknown;
! /*
!* I'm not sure what to do if eob is NULL here -- we can't call
!* PLy_elog because that function calls us, so we could end up
!* with infinite recursion. I'm not even sure if eob could be
!* NULL here -- would an Assert() be more appropriate?
!*/
! estr = eob ? PyString_AsString(eob) : Unknown Exception;
xstr = PLy_printf(%s: %s, estr, vstr);
Py_DECREF(eob);
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] PL/Python error checking
On Mon, Jul 11, 2005 at 08:13:24PM -0600, Michael Fuhr wrote: On Sun, Jul 10, 2005 at 12:58:24AM -0400, Bruce Momjian wrote: I am unclear about backpatching this. We have to weigh the risks of applying or not applying to 8.0.X. Comments? Since 7.4, PL/Python is only available as an untrusted language, so only a database superuser could create an exploitable function. However, it might be possible for an ordinary user to tickle the bug by calling such a function and passing it certain data, either as an argument or as table data. The code is buggy in any case: PyObject_Str() is documented to return NULL on error, and PyString_AsString() doesn't expect a NULL pointer so it segfaults if passed one. Since the patch simply checks for that condition and raises an error instead of calling a function that will segfault and take down the backend, I can't think of what risk applying the patch would have. The greater risk would seem to be in not applying it. I haven't seen this patch applied to other than HEAD. Since it fixes a segmentation fault, should it be backpatched before the new releases? Here's the original patch submission: http://archives.postgresql.org/pgsql-patches/2005-06/msg00519.php -- Michael Fuhr ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] PL/Python error checking
On Sun, Jul 10, 2005 at 12:58:24AM -0400, Bruce Momjian wrote: Michael Fuhr wrote: The patch is against HEAD but the same changes should be applied to earlier versions because they have the same problem. The patch might not apply cleanly against earlier versions -- will the committer take care of little differences or should I submit different versions of the patch? I am unclear about backpatching this. We have to weigh the risks of applying or not applying to 8.0.X. Comments? Since 7.4, PL/Python is only available as an untrusted language, so only a database superuser could create an exploitable function. However, it might be possible for an ordinary user to tickle the bug by calling such a function and passing it certain data, either as an argument or as table data. The code is buggy in any case: PyObject_Str() is documented to return NULL on error, and PyString_AsString() doesn't expect a NULL pointer so it segfaults if passed one. Since the patch simply checks for that condition and raises an error instead of calling a function that will segfault and take down the backend, I can't think of what risk applying the patch would have. The greater risk would seem to be in not applying it. -- Michael Fuhr http://www.fuhr.org/~mfuhr/ ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] PL/Python error checking
Michael Fuhr wrote: This patch addresses the problem mentioned in the process crash when a plpython function returns unicode thread: http://archives.postgresql.org/pgsql-bugs/2005-06/msg00105.php In several places PL/Python was calling PyObject_Str() and then PyString_AsString() without checking if the former had returned NULL to indicate an error. PyString_AsString() doesn't expect a NULL argument, so passing one causes a segmentation fault. This patch adds checks for NULL and raises errors via PLy_elog(), which prints details of the underlying Python exception. The patch also adds regression tests for these checks. All tests pass on my Solaris 9 box running HEAD and Python 2.4.1. In one place the patch doesn't call PLy_elog() because that could cause infinite recursion; see the comment I added. I'm not sure how to test that particular case or whether it's even possible to get an error there: the value that the code should check is the Python exception type, so I wonder if a NULL value shouldn't happen. This patch converts NULL to Unknown Exception but I wonder if an Assert() would be appropriate. The patch is against HEAD but the same changes should be applied to earlier versions because they have the same problem. The patch might not apply cleanly against earlier versions -- will the committer take care of little differences or should I submit different versions of the patch? I am unclear about backpatching this. We have to weigh the risks of applying or not applying to 8.0.X. Comments? -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] PL/Python error checking
Patch applied. Thanks. --- Michael Fuhr wrote: This patch addresses the problem mentioned in the process crash when a plpython function returns unicode thread: http://archives.postgresql.org/pgsql-bugs/2005-06/msg00105.php In several places PL/Python was calling PyObject_Str() and then PyString_AsString() without checking if the former had returned NULL to indicate an error. PyString_AsString() doesn't expect a NULL argument, so passing one causes a segmentation fault. This patch adds checks for NULL and raises errors via PLy_elog(), which prints details of the underlying Python exception. The patch also adds regression tests for these checks. All tests pass on my Solaris 9 box running HEAD and Python 2.4.1. In one place the patch doesn't call PLy_elog() because that could cause infinite recursion; see the comment I added. I'm not sure how to test that particular case or whether it's even possible to get an error there: the value that the code should check is the Python exception type, so I wonder if a NULL value shouldn't happen. This patch converts NULL to Unknown Exception but I wonder if an Assert() would be appropriate. The patch is against HEAD but the same changes should be applied to earlier versions because they have the same problem. The patch might not apply cleanly against earlier versions -- will the committer take care of little differences or should I submit different versions of the patch? -- Michael Fuhr http://www.fuhr.org/~mfuhr/ [ Attachment, skipping... ] ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED] -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 6: explain analyze is your friend
[PATCHES] PL/Python error checking
This patch addresses the problem mentioned in the process crash
when a plpython function returns unicode thread:
http://archives.postgresql.org/pgsql-bugs/2005-06/msg00105.php
In several places PL/Python was calling PyObject_Str() and then
PyString_AsString() without checking if the former had returned
NULL to indicate an error. PyString_AsString() doesn't expect a
NULL argument, so passing one causes a segmentation fault. This
patch adds checks for NULL and raises errors via PLy_elog(), which
prints details of the underlying Python exception. The patch also
adds regression tests for these checks. All tests pass on my
Solaris 9 box running HEAD and Python 2.4.1.
In one place the patch doesn't call PLy_elog() because that could
cause infinite recursion; see the comment I added. I'm not sure
how to test that particular case or whether it's even possible to
get an error there: the value that the code should check is the
Python exception type, so I wonder if a NULL value shouldn't
happen. This patch converts NULL to Unknown Exception but I
wonder if an Assert() would be appropriate.
The patch is against HEAD but the same changes should be applied
to earlier versions because they have the same problem. The patch
might not apply cleanly against earlier versions -- will the committer
take care of little differences or should I submit different versions
of the patch?
--
Michael Fuhr
http://www.fuhr.org/~mfuhr/
Index: src/pl/plpython/plpython.c
===
RCS file: /projects/cvsroot/pgsql/src/pl/plpython/plpython.c,v
retrieving revision 1.62
diff -c -r1.62 plpython.c
*** src/pl/plpython/plpython.c 6 May 2005 17:24:55 - 1.62
--- src/pl/plpython/plpython.c 27 Jun 2005 12:27:06 -
***
*** 517,522
--- 517,524
if (plval != Py_None
!tupdesc-attrs[atti]-attisdropped)
{
plstr = PyObject_Str(plval);
+ if (!plstr)
+ PLy_elog(ERROR, function \%s\ could
not modify tuple, proc-proname);
src = PyString_AsString(plstr);
modvalues[i] =
FunctionCall3(proc-result.out.r.atts[atti].typfunc,
***
*** 774,779
--- 776,783
{
fcinfo-isnull = false;
plrv_so = PyObject_Str(plrv);
+ if (!plrv_so)
+ PLy_elog(ERROR, function \%s\ could not
create return value, proc-proname);
plrv_sc = PyString_AsString(plrv_so);
rv = FunctionCall3(proc-result.out.d.typfunc,
PointerGetDatum(plrv_sc),
***
*** 2019,2025
char *sv;
PyObject *so = PyObject_Str(list);
!
sv = PyString_AsString(so);
PLy_exception_set(PLy_exc_spi_error,
Expected sequence of %d
arguments, got %d. %s,
--- 2023,2031
char *sv;
PyObject *so = PyObject_Str(list);
! if (!so)
! PLy_elog(ERROR, function \%s\ could not execute
plan,
!
PLy_procedure_name(PLy_curr_procedure));
sv = PyString_AsString(so);
PLy_exception_set(PLy_exc_spi_error,
Expected sequence of %d
arguments, got %d. %s,
***
*** 2044,2049
--- 2050,2058
if (elem != Py_None)
{
so = PyObject_Str(elem);
+ if (!so)
+ PLy_elog(ERROR, function \%s\ could
not execute plan,
+
PLy_procedure_name(PLy_curr_procedure));
sv = PyString_AsString(so);
/*
***
*** 2531,2537
else
vstr = Unknown;
! estr = PyString_AsString(eob);
xstr = PLy_printf(%s: %s, estr, vstr);
Py_DECREF(eob);
--- 2540,2552
else
vstr = Unknown;
! /*
!* I'm not sure what to do if eob is NULL here -- we can't call
!* PLy_elog because that function calls us, so we could end up
!* with infinite recursion. I'm not even sure if eob could be
!* NULL here -- would an Assert() be more appropriate?
!*/
! estr = eob ? PyString_AsString(eob) : Unknown Exception;
xstr = PLy_printf(%s: %s, estr, vstr);
Py_DECREF(eob);
Index: src/pl/plpython/expected/plpython_error.out
