From: dan Date: Mon, 24 Aug 2015 19:08:10 +0000 (+0000) Subject: Fix handling of attempts to modify the database schema, application_id or user_versio... X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f5e89dba9d70a4e1b94b8b7b6a519afae839d285;p=thirdparty%2Fsqlite.git Fix handling of attempts to modify the database schema, application_id or user_version within an UNLOCKED transaction. FossilOrigin-Name: 5b9f272113d21fd606903509d6f830fe60fac039 --- diff --git a/manifest b/manifest index 7e91525501..c33005f21e 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Fix\scompilation\swithout\sSQLITE_ENABLE_UNLOCKED.\sAlso\sother\scode\sorganization\sissues. -D 2015-08-24T16:00:08.023 +C Fix\shandling\sof\sattempts\sto\smodify\sthe\sdatabase\sschema,\sapplication_id\sor\suser_version\swithin\san\sUNLOCKED\stransaction. +D 2015-08-24T19:08:10.037 F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f F Makefile.in e2218eb228374422969de7b1680eda6864affcef F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23 @@ -400,7 +400,7 @@ F src/update.c 487747b328b7216bb7f6af0695d6937d5c9e605f F src/utf.c fc6b889ba0779b7722634cdeaa25f1930d93820c F src/util.c bc9dd64b5db544218b871b66243871c202b2781f F src/vacuum.c d35c7291e94a470ee47695c22559a69aef9fdda1 -F src/vdbe.c 97b07a1af65971dab3d326742e912ac8c12108dd +F src/vdbe.c cb555e4a802ec96e16f48c54b57bf15b94616567 F src/vdbe.h 7a75045d879118b9d3af7e8b3c108f2f27c51473 F src/vdbeInt.h 8b54e01ad0463590e7cffabce0bc36da9ee4f816 F src/vdbeapi.c bda74ef4b5103d7b4a4be36f936d3cf2b56a7d6f @@ -411,7 +411,7 @@ F src/vdbesort.c f5009e7a35e3065635d8918b9a31f498a499976b F src/vdbetrace.c 8befe829faff6d9e6f6e4dee5a7d3f85cc85f1a0 F src/vtab.c d31174e4c8f592febab3fa7f69e18320b4fd657a F src/vxworks.h c18586c8edc1bddbc15c004fa16aeb1e1342b4fb -F src/wal.c 7b8c8d1cb09f128d014a1677bc3d3935f729a2f9 +F src/wal.c fd9a3b9fa79042377f3e5e42a91fd6e0daf1037e F src/wal.h 903ef67e17f8b466dc7cfc4186fc23e80be10ff8 F src/walker.c 2e14d17f592d176b6dc879c33fbdec4fbccaa2ba F src/where.c 66518a14a1238611aa0744d6980b6b7f544f4816 @@ -1227,7 +1227,7 @@ F test/types3.test 99e009491a54f4dc02c06bdbc0c5eea56ae3e25a F test/unique.test 93f8b2ef5ea51b9495f8d6493429b1fd0f465264 F test/unique2.test 41e7f83c6827605991160a31380148a9fc5f1339 F test/unixexcl.test cd6c765f75e50e8e2c2ba763149e5d340ea19825 -F test/unlocked.test 51be7f150c8d3bad50b22693e87a91a628e654be +F test/unlocked.test fecdc8d76e3495664c74961e6cd1aef9b59439c1 F test/unlocked2.test 2d969d1b4c3e832169d0c97f522fa5955877785a F test/unlocked3.test 6c99bc0c3f19ad8ca8c1632b09b089164718dc35 F test/unordered.test ca7adce0419e4ca0c50f039885e76ed2c531eda8 @@ -1381,7 +1381,7 @@ F tool/vdbe_profile.tcl 67746953071a9f8f2f668b73fe899074e2c6d8c1 F tool/warnings-clang.sh f6aa929dc20ef1f856af04a730772f59283631d4 F tool/warnings.sh 48bd54594752d5be3337f12c72f28d2080cb630b F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f -P 701302b4bd62ca7aefe643eac096a0ee672a62fa -R 736937b7bbb824036427389285348275 +P 041135575417201bbcf0544cc69dcb7369c7fb34 +R c0f10d85f21cf03acdf9208c7581f1d4 U dan -Z a958ab41b63a16e8331a9ac18c76c8a0 +Z 17437f740072ebd0dda6cab278d01c48 diff --git a/manifest.uuid b/manifest.uuid index 1f1213c7c1..e52ba34ef9 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -041135575417201bbcf0544cc69dcb7369c7fb34 \ No newline at end of file +5b9f272113d21fd606903509d6f830fe60fac039 \ No newline at end of file diff --git a/src/vdbe.c b/src/vdbe.c index b63194655c..3cdb252fdf 100644 --- a/src/vdbe.c +++ b/src/vdbe.c @@ -3227,19 +3227,24 @@ case OP_SetCookie: { /* in3 */ assert( sqlite3SchemaMutexHeld(db, pOp->p1, 0) ); pIn3 = &aMem[pOp->p3]; sqlite3VdbeMemIntegerify(pIn3); +#ifdef SQLITE_ENABLE_UNLOCKED + if( db->bUnlocked + && (pOp->p2==BTREE_USER_VERSION || pOp->p2==BTREE_APPLICATION_ID) + ){ + rc = SQLITE_ERROR; + sqlite3VdbeError(p, "cannot modify %s within UNLOCKED transaction", + pOp->p2==BTREE_USER_VERSION ? "user_version" : "application_id" + ); + break; + } +#endif /* See note about index shifting on OP_ReadCookie */ rc = sqlite3BtreeUpdateMeta(pDb->pBt, pOp->p2, (int)pIn3->u.i); if( pOp->p2==BTREE_SCHEMA_VERSION ){ - if( db->bUnlocked ){ - sqlite3VdbeError(p, "cannot modify database schema - " - "UNLOCKED transaction" - ); - rc = SQLITE_ERROR; - }else{ - /* When the schema cookie changes, record the new cookie internally */ - pDb->pSchema->schema_cookie = (int)pIn3->u.i; - db->flags |= SQLITE_InternChanges; - } + /* When the schema cookie changes, record the new cookie internally */ + assert( db->bUnlocked==0 ); + pDb->pSchema->schema_cookie = (int)pIn3->u.i; + db->flags |= SQLITE_InternChanges; }else if( pOp->p2==BTREE_FILE_FORMAT ){ /* Record changes in the file format */ pDb->pSchema->file_format = (u8)pIn3->u.i; @@ -6110,6 +6115,15 @@ case OP_Expire: { */ case OP_TableLock: { u8 isWriteLock = (u8)pOp->p3; +#ifdef SQLITE_ENABLE_UNLOCKED + if( isWriteLock && db->bUnlocked && pOp->p2==1 ){ + rc = SQLITE_ERROR; + sqlite3VdbeError(p, + "cannot modify database schema within UNLOCKED transaction"); + rc = SQLITE_ERROR; + break; + } +#endif if( isWriteLock || 0==(db->flags&SQLITE_ReadUncommitted) ){ int p1 = pOp->p1; assert( p1>=0 && p1nDb ); diff --git a/src/wal.c b/src/wal.c index 033d33b083..9a2152424a 100644 --- a/src/wal.c +++ b/src/wal.c @@ -1950,25 +1950,14 @@ int sqlite3WalClose( } /* -** Try to read the wal-index header. Return 0 on success and 1 if -** there is a problem. -** -** The wal-index is in shared memory. Another thread or process might -** be writing the header at the same time this procedure is trying to -** read it, which might result in inconsistency. A dirty read is detected -** by verifying that both copies of the header are the same and also by -** a checksum on the header. -** -** If and only if the read is consistent and the header is different from -** pWal->hdr, then pWal->hdr is updated to the content of the new header -** and *pChanged is set to 1. -** -** If the checksum cannot be verified return non-zero. If the header -** is read successfully and the checksum verified, return zero. +** Try to copy the wal-index header from shared-memory into (*pHdr). Return +** zero if successful or non-zero otherwise. If the header is corrupted +** (either because the two copies are inconsistent or because the checksum +** values are incorrect), the read fails and non-zero is returned. */ -static int walIndexTryHdr(Wal *pWal, int *pChanged){ +static int walIndexLoadHdr(Wal *pWal, WalIndexHdr *pHdr){ u32 aCksum[2]; /* Checksum on the header content */ - WalIndexHdr h1, h2; /* Two copies of the header content */ + WalIndexHdr h2; /* Second copy of the header content */ WalIndexHdr volatile *aHdr; /* Header in shared memory */ /* The first page of the wal-index must be mapped at this point. */ @@ -1985,21 +1974,48 @@ static int walIndexTryHdr(Wal *pWal, int *pChanged){ ** reordering the reads and writes. */ aHdr = walIndexHdr(pWal); - memcpy(&h1, (void *)&aHdr[0], sizeof(h1)); + memcpy(pHdr, (void *)&aHdr[0], sizeof(h2)); walShmBarrier(pWal); memcpy(&h2, (void *)&aHdr[1], sizeof(h2)); - if( memcmp(&h1, &h2, sizeof(h1))!=0 ){ + if( memcmp(&h2, pHdr, sizeof(h2))!=0 ){ return 1; /* Dirty read */ } - if( h1.isInit==0 ){ + if( h2.isInit==0 ){ return 1; /* Malformed header - probably all zeros */ } - walChecksumBytes(1, (u8*)&h1, sizeof(h1)-sizeof(h1.aCksum), 0, aCksum); - if( aCksum[0]!=h1.aCksum[0] || aCksum[1]!=h1.aCksum[1] ){ + walChecksumBytes(1, (u8*)&h2, sizeof(h2)-sizeof(h2.aCksum), 0, aCksum); + if( aCksum[0]!=h2.aCksum[0] || aCksum[1]!=h2.aCksum[1] ){ return 1; /* Checksum does not match */ } + return 0; +} + +/* +** Try to read the wal-index header. Return 0 on success and 1 if +** there is a problem. +** +** The wal-index is in shared memory. Another thread or process might +** be writing the header at the same time this procedure is trying to +** read it, which might result in inconsistency. A dirty read is detected +** by verifying that both copies of the header are the same and also by +** a checksum on the header. +** +** If and only if the read is consistent and the header is different from +** pWal->hdr, then pWal->hdr is updated to the content of the new header +** and *pChanged is set to 1. +** +** If the checksum cannot be verified return non-zero. If the header +** is read successfully and the checksum verified, return zero. +*/ +static int walIndexTryHdr(Wal *pWal, int *pChanged){ + WalIndexHdr h1; /* Copy of the header content */ + + if( walIndexLoadHdr(pWal, &h1) ){ + return 1; + } + if( memcmp(&pWal->hdr, &h1, sizeof(WalIndexHdr)) ){ *pChanged = 1; memcpy(&pWal->hdr, &h1, sizeof(WalIndexHdr)); @@ -2636,16 +2652,19 @@ int sqlite3WalLockForCommit(Wal *pWal, PgHdr *pPage1, Bitvec *pAllRead){ ** transaction was started. */ if( rc==SQLITE_OK ){ - volatile WalIndexHdr *pHead; /* Head of the wal file */ - pHead = walIndexHdr(pWal); + WalIndexHdr head; - /* TODO: Check header checksum is good here. */ - - if( memcmp(&pWal->hdr, (void*)pHead, sizeof(WalIndexHdr))!=0 ){ + if( walIndexLoadHdr(pWal, &head) ){ + /* This branch is taken if the wal-index header is corrupted. This + ** occurs if some other writer has crashed while committing a + ** transaction to this database since the current unlocked transaction + ** was opened. */ + rc = SQLITE_BUSY_SNAPSHOT; + }else if( memcmp(&pWal->hdr, (void*)&head, sizeof(WalIndexHdr))!=0 ){ int iHash; - int iLastHash = walFramePage(pHead->mxFrame); + int iLastHash = walFramePage(head.mxFrame); u32 iFirst = pWal->hdr.mxFrame+1; /* First wal frame to check */ - if( memcmp(pWal->hdr.aSalt, (u32*)pHead->aSalt, sizeof(u32)*2) ){ + if( memcmp(pWal->hdr.aSalt, (u32*)head.aSalt, sizeof(u32)*2) ){ assert( pWal->readLock==0 ); iFirst = 1; } @@ -2660,7 +2679,7 @@ int sqlite3WalLockForCommit(Wal *pWal, PgHdr *pPage1, Bitvec *pAllRead){ int iMin = (iFirst - iZero); int iMax = (iHash==0) ? HASHTABLE_NPAGE_ONE : HASHTABLE_NPAGE; if( iMin<1 ) iMin = 1; - if( iMax>pHead->mxFrame ) iMax = pHead->mxFrame; + if( iMax>head.mxFrame ) iMax = head.mxFrame; for(i=iMin; i<=iMax; i++){ PgHdr *pPg; if( aPgno[i]==1 ){ diff --git a/test/unlocked.test b/test/unlocked.test index 5cbb31dbb9..0e22b97fa1 100644 --- a/test/unlocked.test +++ b/test/unlocked.test @@ -110,13 +110,21 @@ foreach {tn trans commit_ok} { foreach {tn sql} { 1 { CREATE TABLE xx(a, b) } 2 { DROP TABLE t1 } + 3 { CREATE INDEX i1 ON t1(a) } + 4 { CREATE VIEW v1 AS SELECT * FROM t1 } + 5 { CREATE TEMP TABLE xx(a, b) } } { do_catchsql_test 1.7.$tn.1 " BEGIN UNLOCKED; $sql - " {1 {cannot modify database schema - UNLOCKED transaction}} + " {1 {cannot modify database schema within UNLOCKED transaction}} - do_execsql_test 1.7.$tn.2 ROLLBACK + do_execsql_test 1.7.$tn.2 { + SELECT sql FROM sqlite_master; + SELECT sql FROM sqlite_temp_master; + } {{CREATE TABLE t1(a, b)}} + + do_execsql_test 1.7.$tn.3 COMMIT } #------------------------------------------------------------------------- @@ -408,6 +416,37 @@ do_multiclient_test tn { do_test 2.$tn.7.4 { sql3 { PRAGMA integrity_check } } ok } - +#------------------------------------------------------------------------- +# Unlocked transactions may not modify the user_version or application_id. +# +reset_db +do_execsql_test 3.0 { + PRAGMA journal_mode = wal; + CREATE TABLE t1(x, y); + INSERT INTO t1 VALUES('a', 'b'); + PRAGMA user_version = 10; +} {wal} +do_execsql_test 3.1 { + BEGIN UNLOCKED; + INSERT INTO t1 VALUES('c', 'd'); + SELECT * FROM t1; +} {a b c d} +do_catchsql_test 3.2 { + PRAGMA user_version = 11; +} {1 {cannot modify user_version within UNLOCKED transaction}} +do_execsql_test 3.3 { + PRAGMA user_version; + SELECT * FROM t1; +} {10 a b c d} +do_catchsql_test 3.4 { + PRAGMA application_id = 11; +} {1 {cannot modify application_id within UNLOCKED transaction}} +do_execsql_test 3.5 { + COMMIT; + PRAGMA user_version; + PRAGMA application_id; + SELECT * FROM t1; +} {10 0 a b c d} finish_test +