New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 626815 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

TabState.saveState will silently fail if contentsState buffer is a MappedByteBuffer (e.g. frozen tabs)

Reported by andrh...@amazon.com, Jul 8 2016

Issue description

Steps 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.
 

Comment 1 by wnwen@chromium.org, Jul 12 2016

Cc: wnwen@chromium.org andrh...@amazon.com
Status: Untriaged (was: Unconfirmed)
Thanks for creating this bug. Chrome 54 should be channel canary, right?
Project Member

Comment 2 by bugdroid1@chromium.org, 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

Comment 3 by cmasso@google.com, Mar 7 2018

Components: UI>Browser
Owner: yus...@chromium.org
Status: Assigned (was: Untriaged)

Sign in to add a comment