Re: [PATCHES] Remove FATAL from pg_lzdecompress
Tom Lane napsal(a): Zdenek Kotala [EMAIL PROTECTED] writes: I attach patch which adds boundaries check and memory overwriting protection when compressed data are corrupted. Good point. Is there plan to applied also on other branch? I wasn't planning to back-patch it. Given the lack of field reports of compressed-data problems, it seemed to me that the risk of breaking something was larger than the chance of helping someone. We could reconsider this after the code has been in HEAD awhile, perhaps. Tom, one of our customer with 3TB table it uses now in production (8.2) awhile (2 weeks) and it works pretty well. He had a corrupted data in TOASTed table and now his system is stable without random crashes. I plan to use this patch in official Solaris build, but I prefer do not have differences between main stream and solaris binaries. Would be possible to backported this patch? Thanks Zdenek -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Remove FATAL from pg_lzdecompress
Tom Lane napsal(a): Zdenek Kotala [EMAIL PROTECTED] writes: I attach patch which adds boundaries check and memory overwriting protection when compressed data are corrupted. Applied with revisions --- it appeared to me that it got the corner case wrong where we find a tag just at the end of the input but there's no room for the output. We'd fall out of the loop and then the error test would think all is well. Good point. Is there plan to applied also on other branch? I think it is useful fix for production release as well. Especially when I want to check all tuples and report tid of corrupted tuples, I'm not able handle FATAL exception. Thanks Zdenek -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Remove FATAL from pg_lzdecompress
Zdenek Kotala [EMAIL PROTECTED] writes: I attach patch which adds boundaries check and memory overwriting protection when compressed data are corrupted. Good point. Is there plan to applied also on other branch? I wasn't planning to back-patch it. Given the lack of field reports of compressed-data problems, it seemed to me that the risk of breaking something was larger than the chance of helping someone. We could reconsider this after the code has been in HEAD awhile, perhaps. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Remove FATAL from pg_lzdecompress
Zdenek Kotala [EMAIL PROTECTED] writes: I attach patch which adds boundaries check and memory overwriting protection when compressed data are corrupted. Applied with revisions --- it appeared to me that it got the corner case wrong where we find a tag just at the end of the input but there's no room for the output. We'd fall out of the loop and then the error test would think all is well. I did not add any extra information into the message. Reasonable solution seems to be use errcontext how was recommended by Alvaro. But I 'm not sure if printtup is good place for it, because pg_detoast is called from many places. However, is can be solved in separate patch. I'm still unconvinced that that's worth any added complexity or slowdown. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Remove FATAL from pg_lzdecompress
Your patch has been added to the PostgreSQL unapplied patches list at: http://momjian.postgresql.org/cgi-bin/pgpatches It will be applied as soon as one of the PostgreSQL committers reviews and approves it. --- Zdenek Kotala wrote: I attach patch which adds boundaries check and memory overwriting protection when compressed data are corrupted. Current behavior let code overwrite a memory and after that check if unpacked size is same as expected value. In this case elog execution fails (at least on Solaris - malloc has corrupted structures) and no message appears in a log file. I did not add any extra information into the message. Reasonable solution seems to be use errcontext how was recommended by Alvaro. But I 'm not sure if printtup is good place for it, because pg_detoast is called from many places. However, is can be solved in separate patch. I'm also think that this modification should be backported to other version too. Thanks Zdenek ---(end of broadcast)--- TIP 6: explain analyze is your friend -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your Subscription: http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches
[PATCHES] Remove FATAL from pg_lzdecompress
I attach patch which adds boundaries check and memory overwriting
protection when compressed data are corrupted.
Current behavior let code overwrite a memory and after that check if
unpacked size is same as expected value. In this case elog execution
fails (at least on Solaris - malloc has corrupted structures) and no
message appears in a log file.
I did not add any extra information into the message. Reasonable
solution seems to be use errcontext how was recommended by Alvaro. But I
'm not sure if printtup is good place for it, because pg_detoast is
called from many places. However, is can be solved in separate patch.
I'm also think that this modification should be backported to other
version too.
Thanks Zdenek
Index: src/backend/utils/adt/pg_lzcompress.c
===
RCS file: /zfs_data/cvs_pgsql/cvsroot/pgsql/src/backend/utils/adt/pg_lzcompress.c,v
retrieving revision 1.29
diff -c -r1.29 pg_lzcompress.c
*** src/backend/utils/adt/pg_lzcompress.c 1 Jan 2008 19:45:52 - 1.29
--- src/backend/utils/adt/pg_lzcompress.c 29 Feb 2008 19:07:50 -
***
*** 633,638
--- 633,639
{
const unsigned char *dp;
const unsigned char *dend;
+ const unsigned char *destend;
unsigned char *bp;
unsigned char ctrl;
int32 ctrlc;
***
*** 643,656
dp = ((const unsigned char *) source) + sizeof(PGLZ_Header);
dend = ((const unsigned char *) source) + VARSIZE(source);
bp = (unsigned char *) dest;
! while (dp dend)
{
/*
* Read one control byte and process the next 8 items.
*/
ctrl = *dp++;
! for (ctrlc = 0; ctrlc 8 dp dend; ctrlc++)
{
if (ctrl 1)
{
--- 644,658
dp = ((const unsigned char *) source) + sizeof(PGLZ_Header);
dend = ((const unsigned char *) source) + VARSIZE(source);
bp = (unsigned char *) dest;
+ destend = ((const unsigned char *)dest) + source-rawsize;
! while (dp dend bp destend)
{
/*
* Read one control byte and process the next 8 items.
*/
ctrl = *dp++;
! for (ctrlc = 0; ctrlc 8 dp dend bp destend; ctrlc++)
{
if (ctrl 1)
{
***
*** 673,678
--- 675,682
* memcpy() here, because the copied areas could overlap
* extremely!
*/
+ if( bp+len destend) /* data are corrupted, do not continue */
+ break;
while (len--)
{
*bp = bp[-off];
***
*** 696,708
}
/*
! * Check we decompressed the right amount, else die. This is a FATAL
! * condition if we tromped on more memory than expected (we assume we have
! * not tromped on shared memory, though, so need not PANIC).
*/
! destsize = (char *) bp - dest;
! if (destsize != source-rawsize)
! elog(destsize source-rawsize ? FATAL : ERROR,
compressed data is corrupt);
/*
--- 700,709
}
/*
! * Check we decompressed the right amount.
*/
! if (bp != destend || dp != dend)
! elog(ERROR,
compressed data is corrupt);
/*
---(end of broadcast)---
TIP 6: explain analyze is your friend
