TabState.saveState will silently fail if contentsState buffer is a MappedByteBuffer (e.g. frozen tabs)
Reported by
andrh...@amazon.com,
Jul 8 2016
|
||
Issue descriptionSteps to reproduce the problem: These steps highlight an unsafe code path in the Tab saving logic. (1) Create a frozen tab and retrieve its tab state via tab.getState(). (2) Open a FileOutputStream on the tab's state file. (3) Call TabState.saveState with the output stream, the tab state retrieved in step 1, and a value of "false" for the "incognito" argument. (4) Reload the saved tab via a call to TabState.restoreState. What is the expected behavior? The tab state is successfully reloaded. What went wrong? The FileChannel.map fails with an "IOException: ftruncate failed" error. Did this work before? N/A Chrome version: 54.0.2786.0 Channel: stable OS Version: 4.4.3 Flash Version: Shockwave Flash 22.0 r0 The TabState.saveState call fails because it attempts to write state.contentsState.buffer() to the output file. However, the .buffer object is of type ByteBuffer and is in some cases an instance of MappedByteBuffer that is mapped to the same output file being written to. For instance, this can happen if the tab being saved is a frozen tab - a use case that is not forbidden/guarded against in the code. In these cases, the write will fail because opening a FileOutputStream on the file will truncate the file's contents and will thus invalidate the memory addresses that the MappedByteBuffer is pointing to. This results in an incomplete save of the tab state, which can lead to failed/incomplete restorations that are difficult to track down.
,
Jul 13 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b6f7a7a894bbced31fb74e75f14364998a610bdc commit b6f7a7a894bbced31fb74e75f14364998a610bdc Author: andrhung <andrhung@amazon.com> Date: Wed Jul 13 21:38:56 2016 Creating contents state byte array before opening stream Changed TabState.saveState to be able to safely write mapped byte buffers by opening FileOutputStream after retrieving buffer contents. Removed unnecessary call to FileChannel.write which is not guaranteed to write all bytes. BUG=626815 Review-Url: https://codereview.chromium.org/2138503002 Cr-Commit-Position: refs/heads/master@{#405299} [modify] https://crrev.com/b6f7a7a894bbced31fb74e75f14364998a610bdc/AUTHORS [modify] https://crrev.com/b6f7a7a894bbced31fb74e75f14364998a610bdc/chrome/android/java/src/org/chromium/chrome/browser/TabState.java [modify] https://crrev.com/b6f7a7a894bbced31fb74e75f14364998a610bdc/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java [modify] https://crrev.com/b6f7a7a894bbced31fb74e75f14364998a610bdc/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersister.java [modify] https://crrev.com/b6f7a7a894bbced31fb74e75f14364998a610bdc/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java [modify] https://crrev.com/b6f7a7a894bbced31fb74e75f14364998a610bdc/chrome/android/java_sources.gni [add] https://crrev.com/b6f7a7a894bbced31fb74e75f14364998a610bdc/chrome/android/junit/src/org/chromium/chrome/browser/tabstate/TabStateUnitTest.java
,
Mar 7 2018
|
||
►
Sign in to add a comment |
||
Comment 1 by wnwen@chromium.org
, Jul 12 2016Status: Untriaged (was: Unconfirmed)