Enable SQLite compile-time flags that disable unused features |
||
Issue descriptionDisabling unused features will reduce the attack surface that we expose to the Web via WebSQL, and give us a smaller binary that is nicer to CPU caches.
,
Jan 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a5f85bc6cde6afb885239a4abff8a6530f4430a9 commit a5f85bc6cde6afb885239a4abff8a6530f4430a9 Author: Victor Costan <pwnall@chromium.org> Date: Tue Jan 30 06:55:28 2018 WebSQL: Explicitly initialize SQLite, remove deprecated API usage. Our WebSQL implementation does not currently call sqlite3_initalize() [1], and instead rely on SQLITE's auto-initialization. The rest of Chromium uses SQLite via //sql/connection.cc, which explicitly initializes SQLite to avoid a race condition. This CL switches WebSQL to explicit sqlite initialization, removing this special case and opening the way for the SQLITE_OMIT_AUTOINIT [2] compile time option. In the interest of reducing IPC latency, our WebSQL implementation embeds SQLite directly in renderer processes, which are sandboxed and don't have direct filesystem access. In order to make this work, we have custom VFS implementations (SQLiteFileSystem{Posix,Win}.cpp). In these cases, we implement xCurrentTime() [3] by delegating to SQLite's VFS implementations. However, xCurrentTime() has been deprecated in favor of xCurrentTimeInt64(). When SQLite is compiled with SQLITE_OMIT_DEPRECATED [4], xCurrentTime() is null, resulting in crashes. This CL upgrades our custom VFS implementations to API version 2, by wrapping xCurrentTimeInt64() instead of wrapping xCurrentTime(). [1] https://www.sqlite.org/c3ref/initialize.html [2] https://www.sqlite.org/compile.html#omit_autoinit [3] https://www.sqlite.org/c3ref/vfs.html [4] https://www.sqlite.org/compile.html#omit_deprecated Bug: 807093 Change-Id: I9775854fd289f30285b589abf44c165715d85841 Reviewed-on: https://chromium-review.googlesource.com/892092 Reviewed-by: Chris Mumford <cmumford@chromium.org> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#532797} [modify] https://crrev.com/a5f85bc6cde6afb885239a4abff8a6530f4430a9/third_party/WebKit/Source/modules/webdatabase/DatabaseTracker.cpp [modify] https://crrev.com/a5f85bc6cde6afb885239a4abff8a6530f4430a9/third_party/WebKit/Source/modules/webdatabase/sqlite/SQLiteFileSystem.cpp [modify] https://crrev.com/a5f85bc6cde6afb885239a4abff8a6530f4430a9/third_party/WebKit/Source/modules/webdatabase/sqlite/SQLiteFileSystem.h [modify] https://crrev.com/a5f85bc6cde6afb885239a4abff8a6530f4430a9/third_party/WebKit/Source/modules/webdatabase/sqlite/SQLiteFileSystemPosix.cpp [modify] https://crrev.com/a5f85bc6cde6afb885239a4abff8a6530f4430a9/third_party/WebKit/Source/modules/webdatabase/sqlite/SQLiteFileSystemWin.cpp
,
Jan 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a0905fab83b6ae86adbec5000872f6b13ec14d33 commit a0905fab83b6ae86adbec5000872f6b13ec14d33 Author: Victor Costan <pwnall@chromium.org> Date: Tue Jan 30 18:48:50 2018 sql: Make //sql/vfs_wrapper.cc compatible with SQLITE_OMIT_DEPRECATED. Our SQLite VFS [1] wrapper implementation unconditionally wraps xCurrentTime(). However, when SQLite is compiled with the SQLITE_OMIT_DEPRECATED option [2], xCurrentTime() is null for the VFS implementations that ship with SQLite, because xCurrentTime() was deprecated in favor of xCurrentTimeInt64(). This CL replaces the unconditional wrapper with a conditional wrapper. This ensures that our VFS wrapper exposes an xCurrentTimeInt64() implementation when used with a modern SQLite library (like the one bundled with Chromium), and an xCurrentTime() implementation when using an older SQLite version (which might be the case when using the library bundled with the operating system). [1] https://www.sqlite.org/c3ref/vfs.html [2] https://www.sqlite.org/compile.html#omit_deprecated Bug: 807093 Change-Id: Id26cc4517a1a23692e7860ed620348c027db990f Reviewed-on: https://chromium-review.googlesource.com/892484 Reviewed-by: Chris Mumford <cmumford@chromium.org> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#532969} [modify] https://crrev.com/a0905fab83b6ae86adbec5000872f6b13ec14d33/sql/vfs_wrapper.cc
,
Jan 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b05f99c3e9e870a4ff9f40960ab81cbd82dc6930 commit b05f99c3e9e870a4ff9f40960ab81cbd82dc6930 Author: Victor Costan <pwnall@chromium.org> Date: Tue Jan 30 19:37:23 2018 sqlite: Disable unused features via compile-time options. This CL shrinks the Linux binary by 34KB (138,266,816 to 138,232,320). Code savings aside, this makes sure we don't unintentionally start depending on these features. The following compile-time options are recommended in the SQLite documentation [1]: * SQLITE_LIKE_DOESNT_MATCH_BLOBS - The LIKE and GLOB operators don't work on BLOB columns. This access pattern would be a performance wrench, so it's nice to fail hard here. * SQLITE_OMIT_DEPRECATED - This removes support for deprecated SQLite APIs. Note that SQLite promises to maintain indefinite backwards compatbility for SQL queries (modulo features disabled via compile-time options), so this only applies to the SQLite API. We should never be using deprecated APIs. * SQLITE_OMIT_PROGRESS_CALLBACK - Remove sqlite3_progress_handler(). We don't use this feature, and the documentation says it comes with a small performance penalty. * SQLITE_OMIT_SHARED_CACHE - Using a shared cache sounds nice from a memory consumption standpoint, and we already do that for LevelDB. Unfortunately, Chromium's SQLite databases use a variety of page sizes, which makes cache sharing impossible. * SQLITE_USE_ALLOCA - Use alloca() instead of malloc() for allocating temporary spaces in functions. All of Chrome's platforms support this option, and it results in a slightly smaller binary and less heap churn. The following compile-time options disable features that happen not to be used. * SQLITE_OMIT_AUTOINIT: We initialize SQLite manually in //sql/connection.cc. * SQLITE_OMIT_AUTORESET: We calls sqlite3_reset() correctly to reset prepared statements. * SQLITE_OMIT_GET_TABLE: We don't use sqlite3_{get,free}_table(). * SQLITE_OMIT_LOAD_EXTENSION: We don't use sqlite3_{enable_}load_extension(). Asides from the code savings, there's a tiny security benefit to knowing that extension loading code is definitely not reachable from WebSQL. * SQLITE_OMIT_TCL_VARIABLE: We don't use TCL variables. * SQLITE_OMIT_TRACE: We don't use sqlite3_{profile,trace}(). Bug: 807093 Change-Id: Ie5e59e55dd9b2ed52f7c27682a809c9f7c36d4a3 Reviewed-on: https://chromium-review.googlesource.com/882961 Commit-Queue: Victor Costan <pwnall@chromium.org> Reviewed-by: Chris Mumford <cmumford@chromium.org> Cr-Commit-Position: refs/heads/master@{#532995} [modify] https://crrev.com/b05f99c3e9e870a4ff9f40960ab81cbd82dc6930/third_party/sqlite/BUILD.gn [modify] https://crrev.com/b05f99c3e9e870a4ff9f40960ab81cbd82dc6930/third_party/sqlite/fuzz/ossfuzz.c [modify] https://crrev.com/b05f99c3e9e870a4ff9f40960ab81cbd82dc6930/third_party/sqlite/fuzz/sqlite3_prepare_v2_fuzzer.cc
,
Jan 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f24fdd9f84945a521665220d062544275932e246 commit f24fdd9f84945a521665220d062544275932e246 Author: Victor Costan <pwnall@chromium.org> Date: Wed Jan 31 00:51:11 2018 Revert "sqlite: Disable unused features via compile-time options." This reverts commit b05f99c3e9e870a4ff9f40960ab81cbd82dc6930. Reason for revert: Crashes the Linux build -- 807487 Original change's description: > sqlite: Disable unused features via compile-time options. > > This CL shrinks the Linux binary by 34KB (138,266,816 to 138,232,320). > Code savings aside, this makes sure we don't unintentionally start > depending on these features. > > The following compile-time options are recommended in the SQLite > documentation [1]: > > * SQLITE_LIKE_DOESNT_MATCH_BLOBS - The LIKE and GLOB operators don't > work on BLOB columns. This access pattern would be a performance > wrench, so it's nice to fail hard here. > * SQLITE_OMIT_DEPRECATED - This removes support for deprecated SQLite > APIs. Note that SQLite promises to maintain indefinite backwards > compatbility for SQL queries (modulo features disabled via > compile-time options), so this only applies to the SQLite API. We > should never be using deprecated APIs. > * SQLITE_OMIT_PROGRESS_CALLBACK - Remove sqlite3_progress_handler(). We > don't use this feature, and the documentation says it comes with a > small performance penalty. > * SQLITE_OMIT_SHARED_CACHE - Using a shared cache sounds nice from a > memory consumption standpoint, and we already do that for LevelDB. > Unfortunately, Chromium's SQLite databases use a variety of page > sizes, which makes cache sharing impossible. > * SQLITE_USE_ALLOCA - Use alloca() instead of malloc() for allocating > temporary spaces in functions. All of Chrome's platforms support this > option, and it results in a slightly smaller binary and less heap churn. > > The following compile-time options disable features that happen not to > be used. > > * SQLITE_OMIT_AUTOINIT: We initialize SQLite manually in //sql/connection.cc. > * SQLITE_OMIT_AUTORESET: We calls sqlite3_reset() correctly to reset prepared > statements. > * SQLITE_OMIT_GET_TABLE: We don't use sqlite3_{get,free}_table(). > * SQLITE_OMIT_LOAD_EXTENSION: We don't use sqlite3_{enable_}load_extension(). > Asides from the code savings, there's a tiny security benefit to > knowing that extension loading code is definitely not reachable from WebSQL. > * SQLITE_OMIT_TCL_VARIABLE: We don't use TCL variables. > * SQLITE_OMIT_TRACE: We don't use sqlite3_{profile,trace}(). > > Bug: 807093 > Change-Id: Ie5e59e55dd9b2ed52f7c27682a809c9f7c36d4a3 > Reviewed-on: https://chromium-review.googlesource.com/882961 > Commit-Queue: Victor Costan <pwnall@chromium.org> > Reviewed-by: Chris Mumford <cmumford@chromium.org> > Cr-Commit-Position: refs/heads/master@{#532995} TBR=cmumford@chromium.org,pwnall@chromium.org Change-Id: Ib49bee7c6c815cafa9eef211c5dd154cd4dd2a8d No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 807093 Reviewed-on: https://chromium-review.googlesource.com/894707 Reviewed-by: Victor Costan <pwnall@chromium.org> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#533121} [modify] https://crrev.com/f24fdd9f84945a521665220d062544275932e246/third_party/sqlite/BUILD.gn [modify] https://crrev.com/f24fdd9f84945a521665220d062544275932e246/third_party/sqlite/fuzz/ossfuzz.c [modify] https://crrev.com/f24fdd9f84945a521665220d062544275932e246/third_party/sqlite/fuzz/sqlite3_prepare_v2_fuzzer.cc
,
Feb 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/42988a99a3129e922c9ea2bda6e7ca9b47498217 commit 42988a99a3129e922c9ea2bda6e7ca9b47498217 Author: Victor Costan <pwnall@chromium.org> Date: Tue Feb 06 02:22:14 2018 sqlite: Remove special cases handling for iOS 9 and below. Chrome currently only supports iOS10 and above. iOS 10 ships with SQLite 3.14.0+, so we can remove some special case handling (and cognitive burden) needed to support older iOS versions. Bug: 807093 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-device Change-Id: I5d19af5eeaffdf587fb43216fc9cf97e9297d15b Reviewed-on: https://chromium-review.googlesource.com/898564 Commit-Queue: Victor Costan <pwnall@chromium.org> Reviewed-by: Marijn Kruisselbrink <mek@chromium.org> Reviewed-by: Chris Mumford <cmumford@chromium.org> Cr-Commit-Position: refs/heads/master@{#534591} [modify] https://crrev.com/42988a99a3129e922c9ea2bda6e7ca9b47498217/content/browser/dom_storage/dom_storage_database_unittest.cc [modify] https://crrev.com/42988a99a3129e922c9ea2bda6e7ca9b47498217/sql/connection_unittest.cc [modify] https://crrev.com/42988a99a3129e922c9ea2bda6e7ca9b47498217/sql/sqlite_features_unittest.cc
,
Feb 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9b4cb224d2ee360861c27dfaefe17b9ae02a25c1 commit 9b4cb224d2ee360861c27dfaefe17b9ae02a25c1 Author: Victor Costan <pwnall@chromium.org> Date: Tue Feb 06 02:54:00 2018 sql: Remove Recovery::FullRecoverySupported(). This always returns true, and the comments around it are misleading. Bug: 807093 Change-Id: Ifb5c7e7e079a09e397123e82dc0659814f4c8a42 Reviewed-on: https://chromium-review.googlesource.com/898464 Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Chris Mumford <cmumford@chromium.org> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#534605} [modify] https://crrev.com/9b4cb224d2ee360861c27dfaefe17b9ae02a25c1/components/history/core/browser/thumbnail_database_unittest.cc [modify] https://crrev.com/9b4cb224d2ee360861c27dfaefe17b9ae02a25c1/components/history/core/browser/top_sites_database_unittest.cc [modify] https://crrev.com/9b4cb224d2ee360861c27dfaefe17b9ae02a25c1/sql/recovery.cc [modify] https://crrev.com/9b4cb224d2ee360861c27dfaefe17b9ae02a25c1/sql/recovery.h
,
Feb 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b3bb8eb0b4064aaf3fa74e762ec964369e8cb651 commit b3bb8eb0b4064aaf3fa74e762ec964369e8cb651 Author: Victor Costan <pwnall@chromium.org> Date: Tue Feb 06 20:18:49 2018 sql: Remove unused iOS-specific header in sql/connection_unittest.cc After https://crrev.com/c/898564, //base/ios/ios_util.h is not needed by SQL::Connection unittests. Bug: 807093 Change-Id: If6736c69dc42d35ecc24a01fc19d43d04b5db7d8 Reviewed-on: https://chromium-review.googlesource.com/903585 Reviewed-by: Chris Mumford <cmumford@chromium.org> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#534772} [modify] https://crrev.com/b3bb8eb0b4064aaf3fa74e762ec964369e8cb651/sql/connection_unittest.cc
,
Feb 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/37558cb100f2fac81e75801d7181610adee71af1 commit 37558cb100f2fac81e75801d7181610adee71af1 Author: Victor Costan <pwnall@chromium.org> Date: Tue Feb 06 20:23:13 2018 sqlite: Prefix "recoverVtableInit" with chrome_sqlite3_. Our custom code for recovering from database corruption uses "recoverVtableInit" as the initialization method. This is in the global namespace. The generic name choice is especially unfortunate in component builds, where this name is seen by the dynamic library loader. This CL turns the name into chrome_sqlite3_recoverVtableInit, which significantly reduces the probability of clashes, and helps trace the name to its source. Bug: 807093 Change-Id: I11fb75a780bf8f432b6a19e58c30fe4b08979760 Reviewed-on: https://chromium-review.googlesource.com/899044 Reviewed-by: Chris Mumford <cmumford@chromium.org> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#534777} [modify] https://crrev.com/37558cb100f2fac81e75801d7181610adee71af1/sql/recovery.cc [modify] https://crrev.com/37558cb100f2fac81e75801d7181610adee71af1/third_party/sqlite/amalgamation/sqlite3.c [modify] https://crrev.com/37558cb100f2fac81e75801d7181610adee71af1/third_party/sqlite/patches/0004-Virtual-table-supporting-recovery-of-corrupted-datab.patch [modify] https://crrev.com/37558cb100f2fac81e75801d7181610adee71af1/third_party/sqlite/src/src/main.c [modify] https://crrev.com/37558cb100f2fac81e75801d7181610adee71af1/third_party/sqlite/src/src/recover.c [modify] https://crrev.com/37558cb100f2fac81e75801d7181610adee71af1/third_party/sqlite/src/src/recover.h
,
Feb 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3d8ec48766b2075fb6f242d13bbc181e850df1fc commit 3d8ec48766b2075fb6f242d13bbc181e850df1fc Author: Victor Costan <pwnall@chromium.org> Date: Tue Feb 06 23:56:15 2018 sqlite: Prefix SQLite API methods with chrome_. In component builds, SQLite's API methods are exported from the chromium_sqlite component, which means they are visible to the dynamic library loader. This opens up the following possibilities: 1) A system library calls into our SQLite instead of calling into the system's SQLite library which it was built against. The patches in our SQLite version lead to different behavior from the system's SQLite, which can cause subtle failures. This happens if the dynamic library loader resolves the system library's symbol imports with our SQLite's exported symbols. 2) A system library loads the system SQLite, and we end up calling into it, instead of calling into our version of SQLite. This happens if the dynamic library loader resolves our symbol imports with the system's SQLite library. Both possibilities above lead to the possibility that the component build will behave differently from the release build, in subtle and potentially non-deterministic ways. This is not a purely academic concern. https://crbug.com/807487 happened because we use NSS on Linux, and NSS invokes SQLite via a complex plugin system. On non-component builds, NSS (a system library) loads and uses the system version of SQLite. On component builds, NSS ends up using our SQLite. This CL fixes the problem by adding a chrome_ prefix to all the symbols exported from SQLite3. In C++ libraries, namespaces can make prefixing easy. Unfortunately, SQLite is a C library, so the prefixing is fairly heavy-handed. A high-level overview of the approach follows: * An extract_sqlite_api Python script reads SQLite's header, extracts the names of all exported symbols, and writes a header file consisting of renaming preprocessor macros, e.g. #define sqlite3_init chrome_sqlite3_init David Benjamin <davidben@chromium.org> designed the approach and wrote the original version of the script. * The script that we use to generate SQLite's amalgamation now also invokes the extract_sqlite_api script described above, and saves the output to amalgamation/rename_exports.h. * The SQLite component exposes an sqlite3.h header that must be used by all SQLite3 users in Chromium. This header now #includes rename_exports.h (containing the renaming preprocessor macros) before #including amalgamation/sqlite3.h. * sqlite3.c (the main output of the amalgamation process) does not #include "sqlite3.h". However, in order to facilitate autoconf builds, it does #include a "config.h", if a certain preprocessor define exists. We abuse that define to have sqlite.c always load config.h, and have config.h load our rename_exports.h. This CL also adds a PRESUBMIT.py that runs unit tests for the extract_sqlite_api Python script, which ensures that the script will not break accidentally. Both the unit tests and the PRESUBIMT script are inspired from //tools/vim. Bug: 807093 , 807487 Change-Id: If3868ba119ffd4ccbb06d1a6fcd4cc2ecd9ef2ae Reviewed-on: https://chromium-review.googlesource.com/898549 Reviewed-by: Chris Mumford <cmumford@chromium.org> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#534843} [modify] https://crrev.com/3d8ec48766b2075fb6f242d13bbc181e850df1fc/third_party/sqlite/BUILD.gn [add] https://crrev.com/3d8ec48766b2075fb6f242d13bbc181e850df1fc/third_party/sqlite/PRESUBMIT.py [modify] https://crrev.com/3d8ec48766b2075fb6f242d13bbc181e850df1fc/third_party/sqlite/README.chromium [delete] https://crrev.com/5157f928d980f7913d23144c597438d4c4f65699/third_party/sqlite/amalgamation/README [add] https://crrev.com/3d8ec48766b2075fb6f242d13bbc181e850df1fc/third_party/sqlite/amalgamation/README.md [modify] https://crrev.com/3d8ec48766b2075fb6f242d13bbc181e850df1fc/third_party/sqlite/amalgamation/config.h [add] https://crrev.com/3d8ec48766b2075fb6f242d13bbc181e850df1fc/third_party/sqlite/amalgamation/rename_exports.h [rename] https://crrev.com/3d8ec48766b2075fb6f242d13bbc181e850df1fc/third_party/sqlite/amalgamation/shell/shell.c [delete] https://crrev.com/5157f928d980f7913d23144c597438d4c4f65699/third_party/sqlite/google_generate_amalgamation.sh [add] https://crrev.com/3d8ec48766b2075fb6f242d13bbc181e850df1fc/third_party/sqlite/scripts/extract_sqlite_api.py [add] https://crrev.com/3d8ec48766b2075fb6f242d13bbc181e850df1fc/third_party/sqlite/scripts/extract_sqlite_api_unittest.py [add] https://crrev.com/3d8ec48766b2075fb6f242d13bbc181e850df1fc/third_party/sqlite/scripts/generate_amalgamation.sh [modify] https://crrev.com/3d8ec48766b2075fb6f242d13bbc181e850df1fc/third_party/sqlite/sqlite3.h
,
Feb 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a02af9067611c9dbe93ace2826eef7e9cd0adb05 commit a02af9067611c9dbe93ace2826eef7e9cd0adb05 Author: Victor Costan <pwnall@chromium.org> Date: Wed Feb 07 02:22:20 2018 sqlite: Disable unused features via compile-time options. This is a reland of b05f99c3e9e870a4ff9f40960ab81cbd82dc6930. Unlike the original, this CL does not enable SQLITE_OMIT_AUTOINIT, to avoid the crashes caused by the problem being addressed in https://crrev.com/c/894692 This CL shrinks the Linux binary by 26KB (139,005,496 to 138,979,176). Code savings aside, this makes sure we don't unintentionally start depending on these features. The following compile-time options are recommended in the SQLite documentation [1]: * SQLITE_LIKE_DOESNT_MATCH_BLOBS - The LIKE and GLOB operators don't work on BLOB columns. This access pattern would be a performance wrench, so it's nice to fail hard here. * SQLITE_OMIT_DEPRECATED - This removes support for deprecated SQLite APIs. Note that SQLite promises to maintain indefinite backwards compatbility for SQL queries (modulo features disabled via compile-time options), so this only applies to the SQLite API. We should never be using deprecated APIs. * SQLITE_OMIT_PROGRESS_CALLBACK - Remove sqlite3_progress_handler(). We don't use this feature, and the documentation says it comes with a small performance penalty. * SQLITE_OMIT_SHARED_CACHE - Using a shared cache sounds nice from a memory consumption standpoint, and we already do that for LevelDB. Unfortunately, Chromium's SQLite databases use a variety of page sizes, which makes cache sharing impossible. * SQLITE_USE_ALLOCA - Use alloca() instead of malloc() for allocating temporary spaces in functions. All of Chrome's platforms support this option, and it results in a slightly smaller binary and less heap churn. The following compile-time options disable features that happen not to be used. * SQLITE_OMIT_AUTORESET: We calls sqlite3_reset() correctly to reset prepared statements. * SQLITE_OMIT_GET_TABLE: We don't use sqlite3_{get,free}_table(). * SQLITE_OMIT_LOAD_EXTENSION: We don't use sqlite3_{enable_}load_extension(). Asides from the code savings, there's a tiny security benefit to knowing that extension loading code is definitely not reachable from WebSQL. * SQLITE_OMIT_TCL_VARIABLE: We don't use TCL variables. * SQLITE_OMIT_TRACE: We don't use sqlite3_{profile,trace}(). [1] https://www.sqlite.org/compile.html#recommended_compile_time_options Bug: 807093 Change-Id: Ic7510b32f96d5d3f98f8c5a9ba522478ad728ad0 Reviewed-on: https://chromium-review.googlesource.com/894708 Commit-Queue: Victor Costan <pwnall@chromium.org> Reviewed-by: Chris Mumford <cmumford@chromium.org> Cr-Commit-Position: refs/heads/master@{#534884} [modify] https://crrev.com/a02af9067611c9dbe93ace2826eef7e9cd0adb05/third_party/sqlite/BUILD.gn [modify] https://crrev.com/a02af9067611c9dbe93ace2826eef7e9cd0adb05/third_party/sqlite/fuzz/ossfuzz.c [modify] https://crrev.com/a02af9067611c9dbe93ace2826eef7e9cd0adb05/third_party/sqlite/fuzz/sqlite3_prepare_v2_fuzzer.cc
,
Feb 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/86ca43119893fc14e1820ac1e2a7e2415cd25643 commit 86ca43119893fc14e1820ac1e2a7e2415cd25643 Author: Victor Costan <pwnall@chromium.org> Date: Wed Feb 07 21:34:10 2018 sqlite: Remove USE_SYSTEM_SQLITE option. We currently use the system-supplied SQLite library on iOS. This is not the end of the world, because we require iOS 10.0+, which means SQLite 3.14+, which has the features that Chrome uses. iOS is also special in that we don't ship Blink, so we don't expose SQLite via WebSQL -- we control all the other SQL queries we send to SQLite, so SQLite security bugs aren't easily exploitable. At the same time, USE_SYSTEM_SQLITE means Chrome for iOS uses an entirely different SQLite build that most developers don't run. This makes it more likely that we'll introduce bugs or crashes, and makes it more difficult to investigate existing bugs/crashes. In theory, USE_SYSTEM_SQLITE seems appealing on Android and Linux. In practice, the idea isn't workable on either platform. * On Android, not shipping our own SQLite would cut down APK size by a few hundred KB. However, Android doesn't have a great upgrade story before Lollipop (5.0), and we still support KitKat (4.4). SQLite is exposed via WebSQL on Android, so we must be in control of SQLite upgrades. * Linux distributions would like to ship Chromium builds linked against their SQLite copies. However, https://crbug.com/807487 suggests that USE_SYSTEM_SQLITE on Linux would result in unacceptable stability. The root cause of the bug was that NSS can load SQLite on Linux, resulting in SQLite getting initialized outside our control. Even if we could fix that bug (via a dependency from //crypto to //sql), we'd still have to chase all the libraries that might potentially initialize SQLite. Today we believe that NSS is the only library in that set, but we simply don't have the bandwidth to justify investigating the bug reports that will come if we miss something. In conclusion, there is no platform where USE_SYSTEM_SQLITE is desirable and viable. So, this CL removes USE_SYSTEM_SQLITE from the codebase. This will cause the iOS build to increase by a few hundred KB. This price buys us one less difference between iOS and every other platform, and increased OS stability in the long run, as iOS bugs / crashes will be easier to diagnose. Bug: 22208 , 299684 , 372584, 807093 , 807487 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-device Change-Id: I1bd64bd93527624e197bd082c725a952c8b3c430 Reviewed-on: https://chromium-review.googlesource.com/898223 Commit-Queue: Victor Costan <pwnall@chromium.org> Reviewed-by: Chris Mumford <cmumford@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Cr-Commit-Position: refs/heads/master@{#535148} [modify] https://crrev.com/86ca43119893fc14e1820ac1e2a7e2415cd25643/sql/connection_unittest.cc [modify] https://crrev.com/86ca43119893fc14e1820ac1e2a7e2415cd25643/sql/recovery.cc [modify] https://crrev.com/86ca43119893fc14e1820ac1e2a7e2415cd25643/sql/sqlite_features_unittest.cc [modify] https://crrev.com/86ca43119893fc14e1820ac1e2a7e2415cd25643/sql/vfs_wrapper.cc [modify] https://crrev.com/86ca43119893fc14e1820ac1e2a7e2415cd25643/third_party/sqlite/BUILD.gn [modify] https://crrev.com/86ca43119893fc14e1820ac1e2a7e2415cd25643/third_party/sqlite/sqlite3.h [modify] https://crrev.com/86ca43119893fc14e1820ac1e2a7e2415cd25643/tools/metrics/histograms/enums.xml
,
Feb 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9adf73a5d0aa3959d3d8bb1e062a88a08d7c117d commit 9adf73a5d0aa3959d3d8bb1e062a88a08d7c117d Author: Victor Costan <pwnall@chromium.org> Date: Thu Feb 08 18:29:58 2018 sqlite: Enable SQLITE_OMIT_AUTORESET. This was first attemped in https://crrev.com/c/882961, but broke the component build used by developers -- https://crbug.com/807487 . Re-landing this option is now safe thanks to https://crrev.com/c/898549 and https://crrev.com/c/898223 This CL saves 4KB of binary size (139,280,808 -> 139,276,712 on Linux). Bug: 807093 Change-Id: I2c28c4901d213bf375aad684d04b9f1c2857f90c Reviewed-on: https://chromium-review.googlesource.com/907866 Reviewed-by: Chris Mumford <cmumford@chromium.org> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#535448} [modify] https://crrev.com/9adf73a5d0aa3959d3d8bb1e062a88a08d7c117d/third_party/sqlite/BUILD.gn
,
Feb 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3653df64aefea8e3ebef0976e82bd0d8918effaa commit 3653df64aefea8e3ebef0976e82bd0d8918effaa Author: Victor Costan <pwnall@chromium.org> Date: Thu Feb 08 21:38:16 2018 WebSQL: Initialize SQLite via //sql. Chrome uses SQLite via the high-level API in //sql, with the notable exception of Blink's WebSQL implementation. This is for historical reasons -- WebSQL was implemented before the Blink/WebKit fork, so it was unacceptable for the WebSQL implementation to depend on code in the Chromium source tree. In the long run (assuming that WebSQL doesn't go away completely) we want WebSQL to also use SQLite via //sql. This CL makes the first step, by routing SQLite initialization via //sql, instead of calling the library directly. This CL also fixes up Blink's dependencies on SQLite. Blink core does not depend on SQLite, and the only module depending on it is WebSQL. Bug: 807093 Change-Id: Iae6a02e5b0488051853f75dcb397e7c68ffc961b Reviewed-on: https://chromium-review.googlesource.com/894692 Commit-Queue: Victor Costan <pwnall@chromium.org> Reviewed-by: Chris Mumford <cmumford@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#535528} [modify] https://crrev.com/3653df64aefea8e3ebef0976e82bd0d8918effaa/PRESUBMIT.py [modify] https://crrev.com/3653df64aefea8e3ebef0976e82bd0d8918effaa/sql/BUILD.gn [modify] https://crrev.com/3653df64aefea8e3ebef0976e82bd0d8918effaa/sql/connection.cc [add] https://crrev.com/3653df64aefea8e3ebef0976e82bd0d8918effaa/sql/initialization.cc [add] https://crrev.com/3653df64aefea8e3ebef0976e82bd0d8918effaa/sql/initialization.h [modify] https://crrev.com/3653df64aefea8e3ebef0976e82bd0d8918effaa/third_party/WebKit/Source/core/BUILD.gn [modify] https://crrev.com/3653df64aefea8e3ebef0976e82bd0d8918effaa/third_party/WebKit/Source/modules/BUILD.gn [modify] https://crrev.com/3653df64aefea8e3ebef0976e82bd0d8918effaa/third_party/WebKit/Source/modules/webdatabase/BUILD.gn [modify] https://crrev.com/3653df64aefea8e3ebef0976e82bd0d8918effaa/third_party/WebKit/Source/modules/webdatabase/sqlite/DEPS [modify] https://crrev.com/3653df64aefea8e3ebef0976e82bd0d8918effaa/third_party/WebKit/Source/modules/webdatabase/sqlite/SQLiteFileSystem.cpp [modify] https://crrev.com/3653df64aefea8e3ebef0976e82bd0d8918effaa/third_party/WebKit/Tools/Scripts/audit-non-blink-usage.py
,
Feb 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ed7422bd703d7ca5f74c2f7f4e4147612c466fd4 commit ed7422bd703d7ca5f74c2f7f4e4147612c466fd4 Author: Victor Costan <pwnall@chromium.org> Date: Sat Feb 10 00:35:05 2018 sqlite: Updated TODO owners and version numbers. Additionally minor cleanup of upgrade steps in README.chromium. This is a follow-up addressing some feedback received in the reviews for the attached bug. Bug: 807093 Change-Id: Idd5e4fb196aeac6c101c1773ac6a0c364bb73ec3 Reviewed-on: https://chromium-review.googlesource.com/907868 Reviewed-by: Chris Mumford <cmumford@chromium.org> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#535900} [modify] https://crrev.com/ed7422bd703d7ca5f74c2f7f4e4147612c466fd4/third_party/sqlite/BUILD.gn [modify] https://crrev.com/ed7422bd703d7ca5f74c2f7f4e4147612c466fd4/third_party/sqlite/README.chromium
,
Feb 10 2018
,
Feb 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b482d796009843373dbc38abb2da6134489a668f commit b482d796009843373dbc38abb2da6134489a668f Author: Victor Costan <pwnall@chromium.org> Date: Tue Feb 20 23:37:26 2018 sqlite: Use SQLITE_ENABLE_API_ARMOR when DCHECK is on. SQLITE_ENABLE_API_ARMOR [1] adds checks against SQLite API misuse. This flag is equivalent to DCHECKs that check preconditions at the begining of an API call, so it should be enabled in builds where DCHECKs are enabled. [1] https://sqlite.org/compile.html#enable_api_armor Bug: 807093 , 812589 Change-Id: I97721723334bd7bc4166971e05f6f032d7376abc Cq-Include-Trybots: master.tryserver.chromium.win:win_chrome_official;master.tryserver.chromium.linux:linux_chromium_msan_rel_ng;master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng;master.tryserver.chromium.mac:mac_chromium_asan_rel_ng Reviewed-on: https://chromium-review.googlesource.com/924565 Reviewed-by: Chris Mumford <cmumford@chromium.org> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#537934} [modify] https://crrev.com/b482d796009843373dbc38abb2da6134489a668f/third_party/sqlite/BUILD.gn [modify] https://crrev.com/b482d796009843373dbc38abb2da6134489a668f/third_party/sqlite/amalgamation/sqlite3.c [add] https://crrev.com/b482d796009843373dbc38abb2da6134489a668f/third_party/sqlite/patches/0011-Fix-thread-sanitizer-TSAN-error-when-SQLITE_ENABLE_A.patch [modify] https://crrev.com/b482d796009843373dbc38abb2da6134489a668f/third_party/sqlite/src/src/mutex_unix.c |
||
►
Sign in to add a comment |
||
Comment 1 by bugdroid1@chromium.org
, Jan 30 2018