From 112f752be8693eb23e848a54497c963c32ac05d4 Mon Sep 17 00:00:00 2001 From: danielk1977 Date: Thu, 8 Jan 2009 17:50:45 +0000 Subject: [PATCH] Fix a couple of potential corruption problems in pager.c. (CVS 6143) FossilOrigin-Name: 5a39525ba3e65f1c6df3cf23fbfb57f6a0bf4b62 --- manifest | 15 ++++--- manifest.uuid | 2 +- src/pager.c | 43 +++++++++++++++--- test/crash8.test | 112 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 159 insertions(+), 13 deletions(-) create mode 100644 test/crash8.test diff --git a/manifest b/manifest index bec3c59030..1b7559df56 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Add\sa\stest\sscript\sfor\sticket\s#2565.\s\sChange\sthe\sassert()\sin\spager.c\sinto\na\stestcase()\smacro.\s(CVS\s6142) -D 2009-01-08T15:24:02 +C Fix\sa\scouple\sof\spotential\scorruption\sproblems\sin\spager.c.\s(CVS\s6143) +D 2009-01-08T17:50:46 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0 F Makefile.in 05461a9b5803d5ad10c79f989801e9fd2cc3e592 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654 @@ -142,7 +142,7 @@ F src/os_common.h 24525d8b7bce66c374dfc1810a6c9043f3359b60 F src/os_os2.c bed77dc26e3a95ce4a204936b9a1ca6fe612fcc5 F src/os_unix.c a1f05f59c24e61186c981f2a7fea13986db620f1 F src/os_win.c 496e3ceb499aedc63622a89ef76f7af2dd902709 -F src/pager.c 2b35462d94ef080eb21902ba4747593ef261069f +F src/pager.c 35445d2172eb39c47ec146777c26e47e6d865538 F src/pager.h 9870acb2d653848d90d765d7cbf163496d6c8111 F src/parse.y 4d0e33a702dc3ea7b69d8ae1914b3fbd32e46057 F src/pcache.c 16dc8da6e6ba6250f8dfd9ee46036db1cbceedc6 @@ -288,6 +288,7 @@ F test/crash4.test 02ff4f15c149ca1e88a5c299b4896c84d9450c3b F test/crash5.test 80a2f7073381837fc082435c97df52a830abcd80 F test/crash6.test 9c730cf06335003cb1f5cfceddacd044155336e0 F test/crash7.test e20a7b9ee1d16eaef7c94a4cb7ed2191b4d05970 +F test/crash8.test 8a7134b3b51e6914054ecd47115a62d9be6726da F test/crashtest1.c 09c1c7d728ccf4feb9e481671e29dda5669bbcc2 F test/createtab.test 199cf68f44e5d9e87a0b8afc7130fdeb4def3272 F test/cse.test 277350a26264495e86b1785f34d2d0c8600e021c @@ -695,7 +696,7 @@ F tool/speedtest16.c c8a9c793df96db7e4933f0852abb7a03d48f2e81 F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e -P 81014334ad57e380e21c47ad6eebe9f16b4ad24c -R 1fb9e89475372962c965501bf1119839 -U drh -Z f5bb78d1eaf6298f714991d532608b85 +P 1e53e382e5030de4d908098bf77af053dd81f0c5 +R 124292e0acdcfa03ab956f6d28184270 +U danielk1977 +Z 35376a8787a84eed4d896203061ddd89 diff --git a/manifest.uuid b/manifest.uuid index 3a243b6a65..5942436ec0 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -1e53e382e5030de4d908098bf77af053dd81f0c5 \ No newline at end of file +5a39525ba3e65f1c6df3cf23fbfb57f6a0bf4b62 \ No newline at end of file diff --git a/src/pager.c b/src/pager.c index 08075058dd..6a65c09e8c 100644 --- a/src/pager.c +++ b/src/pager.c @@ -18,7 +18,7 @@ ** file simultaneously, or one process from reading the database while ** another is writing. ** -** @(#) $Id: pager.c,v 1.541 2009/01/08 15:24:02 drh Exp $ +** @(#) $Id: pager.c,v 1.542 2009/01/08 17:50:46 danielk1977 Exp $ */ #ifndef SQLITE_OMIT_DISKIO #include "sqliteInt.h" @@ -580,7 +580,7 @@ static int readMasterJournal(sqlite3_file *pJrnl, char *zMaster, u32 nMaster){ ** 2000 2048 ** */ -static void seekJournalHdr(Pager *pPager){ +static i64 journalHdrOffset(Pager *pPager){ i64 offset = 0; i64 c = pPager->journalOff; if( c ){ @@ -589,7 +589,10 @@ static void seekJournalHdr(Pager *pPager){ assert( offset%JOURNAL_HDR_SZ(pPager)==0 ); assert( offset>=c ); assert( (offset-c)journalOff = offset; + return offset; +} +static void seekJournalHdr(Pager *pPager){ + pPager->journalOff = journalHdrOffset(pPager); } /* @@ -2487,6 +2490,35 @@ static int syncJournal(Pager *pPager){ assert( pPager->journalOpen ); if( 0==(iDc&SQLITE_IOCAP_SAFE_APPEND) ){ + i64 jrnlOff = journalHdrOffset(pPager); + u8 zMagic[8]; + + /* This block deals with an obscure problem. If the last connection + ** that wrote to this database was operating in persistent-journal + ** mode, then the journal file may at this point actually be larger + ** than Pager.journalOff bytes. If the next thing in the journal + ** file happens to be a journal-header (written as part of the + ** previous connections transaction), and a crash or power-failure + ** occurs after nRec is updated but before this connection writes + ** anything else to the journal file (or commits/rolls back its + ** transaction), then SQLite may become confused when doing the + ** hot-journal rollback following recovery. It may roll back all + ** of this connections data, then proceed to rolling back the old, + ** out-of-date data that follows it. Database corruption. + ** + ** To work around this, if the journal file does appear to contain + ** a valid header following Pager.journalOff, then write a 0x00 + ** byte to the start of it to prevent it from being recognized. + */ + rc = sqlite3OsRead(pPager->jfd, zMagic, 8, jrnlOff); + if( rc==SQLITE_OK && 0==memcmp(zMagic, aJournalMagic, 8) ){ + static const u8 zerobyte = 0; + rc = sqlite3OsWrite(pPager->jfd, &zerobyte, 1, jrnlOff); + } + if( rc!=SQLITE_OK && rc!=SQLITE_IOERR_SHORT_READ ){ + return rc; + } + /* Write the nRec value into the journal file header. If in ** full-synchronous mode, sync the journal first. This ensures that ** all data has really hit the disk before nRec is updated to mark @@ -2498,7 +2530,6 @@ static int syncJournal(Pager *pPager){ ** is populated with 0xFFFFFFFF when the journal header is written ** and never needs to be updated. */ - i64 jrnlOff; if( pPager->fullSync && 0==(iDc&SQLITE_IOCAP_SEQUENTIAL) ){ PAGERTRACE(("SYNC journal of %d\n", PAGERID(pPager))); IOTRACE(("JSYNC %p\n", pPager)) @@ -2879,8 +2910,10 @@ static int pagerSharedLock(Pager *pPager){ pPager->journalHdr = 0; /* Playback and delete the journal. Drop the database write - ** lock and reacquire the read lock. + ** lock and reacquire the read lock. Purge the cache before + ** playing back the hot-journal so that we don't end up with */ + sqlite3PcacheClear(pPager->pPCache); rc = pager_playback(pPager, 1); if( rc!=SQLITE_OK ){ rc = pager_error(pPager, rc); diff --git a/test/crash8.test b/test/crash8.test new file mode 100644 index 0000000000..97706ab8f6 --- /dev/null +++ b/test/crash8.test @@ -0,0 +1,112 @@ +# 2009 January 8 +# +# The author disclaims copyright to this source code. In place of +# a legal notice, here is a blessing: +# +# May you do good and not evil. +# May you find forgiveness for yourself and forgive others. +# May you share freely, never taking more than you give. +# +#*********************************************************************** +# +# This test verifies a couple of specific potential data corruption +# scenarios involving crashes or power failures. +# +# $Id: crash8.test,v 1.1 2009/01/08 17:50:46 danielk1977 Exp $ + + +set testdir [file dirname $argv0] +source $testdir/tester.tcl + +ifcapable !crashtest { + finish_test + return +} + +do_test crash8-1.1 { + execsql { + CREATE TABLE t1(a, b); + CREATE INDEX i1 ON t1(a, b); + INSERT INTO t1 VALUES(1, randstr(1000,1000)); + INSERT INTO t1 VALUES(2, randstr(1000,1000)); + INSERT INTO t1 VALUES(3, randstr(1000,1000)); + INSERT INTO t1 VALUES(4, randstr(1000,1000)); + INSERT INTO t1 VALUES(5, randstr(1000,1000)); + INSERT INTO t1 VALUES(6, randstr(1000,1000)); + CREATE TABLE t2(a, b); + CREATE TABLE t3(a, b); + CREATE TABLE t4(a, b); + CREATE TABLE t5(a, b); + CREATE TABLE t6(a, b); + CREATE TABLE t7(a, b); + CREATE TABLE t8(a, b); + CREATE TABLE t9(a, b); + CREATE TABLE t10(a, b); + PRAGMA integrity_check + } +} {ok} + + +# Potential corruption scenario 1. A second process opens the database +# and modifies a large portion of it. It then opens a second transaction +# and modifies a small part of the database, but crashes before it commits +# the transaction. +# +# When the first process accessed the database again, it was rolling back +# the aborted transaction, but was not purging its in-memory cache (which +# was loaded before the second process made its first, successful, +# modification). Producing an inconsistent cache. +# +do_test crash8-1.2 { + crashsql -delay 2 -file test.db { + PRAGMA cache_size = 10; + UPDATE t1 SET b = randstr(1000,1000); + INSERT INTO t9 VALUES(1, 2); + } +} {1 {child process exited abnormally}} +do_test crash8-1.3 { + execsql {PRAGMA integrity_check} +} {ok} + +# Potential corruption scenario 2. The second process, operating in +# persistent-journal mode, makes a large change to the database file +# with a small in-memory cache. Such that more than one journal-header +# was written to the file. It then opens a second transaction and makes +# a smaller change that requires only a single journal-header to be +# written to the journal file. The second change is such that the +# journal content written to the persistent journal file exactly overwrites +# the first journal-header and set of subsequent records written by the +# first, successful, change. The second process crashes before it can +# commit its second change. +# +# When the first process accessed the database again, it was rolling back +# the second aborted transaction, then continuing to rollback the second +# and subsequent journal-headers written by the first, successful, change. +# Database corruption. +# +do_test crash8.2.1 { + crashsql -delay 2 -file test.db { + PRAGMA journal_mode = persist; + PRAGMA cache_size = 10; + UPDATE t1 SET b = randstr(1000,1000); + PRAGMA cache_size = 100; + BEGIN; + INSERT INTO t2 VALUES('a', 'b'); + INSERT INTO t3 VALUES('a', 'b'); + INSERT INTO t4 VALUES('a', 'b'); + INSERT INTO t5 VALUES('a', 'b'); + INSERT INTO t6 VALUES('a', 'b'); + INSERT INTO t7 VALUES('a', 'b'); + INSERT INTO t8 VALUES('a', 'b'); + INSERT INTO t9 VALUES('a', 'b'); + INSERT INTO t10 VALUES('a', 'b'); + COMMIT; + } +} {1 {child process exited abnormally}} + +do_test crash8-2.3 { + execsql {PRAGMA integrity_check} +} {ok} + +finish_test + -- 2.47.2