Thread: How to crash postgres using savepoints

How to crash postgres using savepoints

From
Christopher Kings-Lynne
Date:
test=# begin;
BEGIN
test=# savepoint "A";
SAVEPOINT
test=# rollback to a;
server closed the connection unexpectedly        This probably means the server terminated abnormally        before or
whileprocessing the request.
 
LOG:  server process (PID 45905) was terminated by signal 11
LOG:  terminating any other active server processes
The connection to the server was lost. Attempting reset: LOG: 
background writer process (PID 45899) exited with exit code 1
LOG:  all server processes terminated; reinitializing
LOG:  database system was interrupted at 2004-08-03 09:42:11 WST
LOG:  checkpoint record is at 0/A7067C

Chris


Re: How to crash postgres using savepoints

From
Christopher Kings-Lynne
Date:
Did this get through?  Hadn't seen anyone comment on it, and I thought 
it was pretty major :P

Christopher Kings-Lynne wrote:

> test=# begin;
> BEGIN
> test=# savepoint "A";
> SAVEPOINT
> test=# rollback to a;
> server closed the connection unexpectedly
>         This probably means the server terminated abnormally
>         before or while processing the request.
> LOG:  server process (PID 45905) was terminated by signal 11
> LOG:  terminating any other active server processes
> The connection to the server was lost. Attempting reset: LOG: background 
> writer process (PID 45899) exited with exit code 1
> LOG:  all server processes terminated; reinitializing
> LOG:  database system was interrupted at 2004-08-03 09:42:11 WST
> LOG:  checkpoint record is at 0/A7067C
> 
> Chris
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org


Re: How to crash postgres using savepoints

From
Jeff Davis
Date:
On Mon, 2004-08-02 at 22:37, Christopher Kings-Lynne wrote:
> Did this get through?  Hadn't seen anyone comment on it, and I thought 
> it was pretty major :P
> 

I'd just like to second your claims. I have a snapshot from 2004-08-02
and I appended a sequence of SQL commands that causes a crash for me.

Regards,    Jeff Davis

Sequence that causes crash:

test=# begin;
BEGIN
test=# savepoint a;
SAVEPOINT
test=# rollback to b;
server closed the connection unexpectedly       This probably means the server terminated abnormally       before or
whileprocessing the request.
 
The connection to the server was lost. Attempting reset: LOG:  server
process (PID 23751) was terminated by signal 11
LOG:  terminating any other active server processes
LOG:  background writer process (PID 20334) exited with exit code 1
LOG:  all server processes terminated; reinitializing
LOG:  database system was interrupted at 2004-08-02 21:40:58 PDT
LOG:  checkpoint record is at 0/A70914
LOG:  redo record is at 0/A70914; undo record is at 0/0; shutdown TRUE
LOG:  next transaction ID: 529; next OID: 25420
LOG:  database system was not properly shut down; automatic recovery in
progressLOG:  record with zero length at 0/A70954
LOG:  redo is not required
LOG:  database system is ready
Succeeded.
test=#






Re: How to crash postgres using savepoints

From
Gavin Sherry
Date:
Attached is a patch fixing this.

One question I do have:
if (target->savepointLevel != s->savepointLevel)

Will this ever be true in the current code? I cannot see anything setting
savepointLevel explicitly.

Gavin

Re: How to crash postgres using savepoints

From
Jeff Davis
Date:
On Tue, 2004-08-03 at 03:41, Gavin Sherry wrote:
> Attached is a patch fixing this.
> 
> One question I do have:
> 
>     if (target->savepointLevel != s->savepointLevel)
> 
> Will this ever be true in the current code? I cannot see anything setting
> savepointLevel explicitly.

>From reading the lists, it seems like that's allowing for some
functionality that was talked about but wasn't part of the semantics
agreed upon by the end of the discussion.

I have a question for you also. I just posted a patch at about the same
time you did (I sent it to pgsql-patches, but I haven't seen it appear
yet). Mine was a one-liner (appended to end of this email) and all it
did was add a check into the aforementioned line for a non-null target
pointer. My patch seemed to work, so I'd like to know why you changed
the structure around more. I did notice some things were a little
cleaner, so was it just clean-up or does my patch fail in some way?

Regards,Jeff

--- xact.c.old  2004-08-03 03:18:12.000000000 -0700
+++ xact.c      2004-08-03 03:19:05.000000000 -0700
@@ -2529,7 +2529,7 @@               target = target->parent;
               /* we don't cross savepoint level boundaries */
-               if (target->savepointLevel != s->savepointLevel)
+               if (PointerIsValid(target) && (target->savepointLevel !=
s->savepointLevel))                       ereport(ERROR,                                      
(errcode(ERRCODE_S_E_INVALID_SPECIFICATION),                                        errmsg("no such savepoint")));




Re: How to crash postgres using savepoints

From
Gavin Sherry
Date:
On Tue, 3 Aug 2004, Jeff Davis wrote:

> On Tue, 2004-08-03 at 03:41, Gavin Sherry wrote:
> > Attached is a patch fixing this.
> >
> > One question I do have:
> >
> >     if (target->savepointLevel != s->savepointLevel)
> >
> > Will this ever be true in the current code? I cannot see anything setting
> > savepointLevel explicitly.
>
> >From reading the lists, it seems like that's allowing for some
> functionality that was talked about but wasn't part of the semantics
> agreed upon by the end of the discussion.

Yes. Savepoint levels are covered in the spec I was more or less just
wondering if there was any more code to come or if I'd missed something.

>
> I have a question for you also. I just posted a patch at about the same
> time you did (I sent it to pgsql-patches, but I haven't seen it appear
> yet). Mine was a one-liner (appended to end of this email) and all it
> did was add a check into the aforementioned line for a non-null target
> pointer. My patch seemed to work, so I'd like to know why you changed
> the structure around more. I did notice some things were a little
> cleaner, so was it just clean-up or does my patch fail in some way?

Just a clean up.

Gavin


Re: How to crash postgres using savepoints

From
Tom Lane
Date:
Gavin Sherry <swm@linuxworld.com.au> writes:
> On Tue, 3 Aug 2004, Jeff Davis wrote:
>> I have a question for you also. I just posted a patch at about the same
>> time you did (I sent it to pgsql-patches, but I haven't seen it appear
>> yet). Mine was a one-liner (appended to end of this email) and all it
>> did was add a check into the aforementioned line for a non-null target
>> pointer. My patch seemed to work, so I'd like to know why you changed
>> the structure around more. I did notice some things were a little
>> cleaner, so was it just clean-up or does my patch fail in some way?

> Just a clean up.

Actually, there's an easier fix than either of these, which is just to
pull the savepointLevel test out of the loop and only do it after we've
found a candidate savepoint.  There's no point in checking the level at
every loop iteration (especially since the feature isn't even being used
yet, as noted).

Attached is the patch I just applied (which also includes my own notions
about how to make the loop prettier...)
        regards, tom lane


*** src/backend/access/transam/xact.c.orig    Sun Aug  1 16:57:59 2004
--- src/backend/access/transam/xact.c    Tue Aug  3 11:53:41 2004
***************
*** 2520,2541 ****      Assert(PointerIsValid(name)); 
!     target = CurrentTransactionState;
! 
!     while (target != NULL)     {         if (PointerIsValid(target->name) && strcmp(target->name, name) == 0)
   break;
 
-         target = target->parent;
- 
-         /* we don't cross savepoint level boundaries */
-         if (target->savepointLevel != s->savepointLevel)
-             ereport(ERROR,
-                     (errcode(ERRCODE_S_E_INVALID_SPECIFICATION),
-                      errmsg("no such savepoint")));     }      if (!PointerIsValid(target))         ereport(ERROR,
            (errcode(ERRCODE_S_E_INVALID_SPECIFICATION),                  errmsg("no such savepoint")));
 
--- 2520,2538 ----      Assert(PointerIsValid(name)); 
!     for (target = s; PointerIsValid(target); target = target->parent)     {         if (PointerIsValid(target->name)
&&strcmp(target->name, name) == 0)             break;     }      if (!PointerIsValid(target))
 
+         ereport(ERROR,
+                 (errcode(ERRCODE_S_E_INVALID_SPECIFICATION),
+                  errmsg("no such savepoint")));
+ 
+     /* disallow crossing savepoint level boundaries */
+     if (target->savepointLevel != s->savepointLevel)         ereport(ERROR,
(errcode(ERRCODE_S_E_INVALID_SPECIFICATION),                 errmsg("no such savepoint")));