New issue
Advanced search Search tips

Issue 642247 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Dynamic refresh of the read-only policy for external storages

Project Member Reported by yamaguchi@chromium.org, Aug 30 2016

Issue description

Chrome OS Version: ToT

See Issue 629945 for the details of the read-only policy.

Currently the read-only policy is applied at the next time each device is remounted. (e.g. either "resume from suspend", re-plugging a device, or re-login) Change it to reflect as soon as the device receives the profile change.
We will be able to do in the same way as we do for ExternalStorageDisabled.

Caveats:
When an user is reading a large file in read-write mode, we may want to postpone re-mounting the disk in ro mode until all I/O's finish, as reading operation in this case is not violating the policy and should not be aborted.


Steps To Reproduce:
(1) Apply the read-only policy for external storage
   ExternalStorageReadOnly=true
   ExternalStorageDisabled=false
(1') or alternatively, apply patch  https://codereview.chromium.org/2248033003/
(2) Plug a USB drive
(3) Change the policy to allow writing to external devices
   ExternalStorageReadOnly=false
   ExternalStorageDisabled=false
(4) After a few seconds, see if the disk is writable

Expected Result:
The disk will become writable without any action.

Actual Result:
Not writable.

How frequently does this problem reproduce? (Always, sometimes, hard to
reproduce?)
Always

What is the impact to the user, and is there a workaround? If so, what is
it?
It would be more convenient and easier to understand, if the policy update is applied without taking any actions.
 
Cc: sdurais...@google.com
Labels: M-55
Status: Started (was: Unconfirmed)
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/b670bd19f986b0a30eb5ef84a73bbab175192ffe

commit b670bd19f986b0a30eb5ef84a73bbab175192ffe
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Wed Sep 28 14:06:44 2016

cros-disks: Implement Remount method.

BUG= chromium:642247 
TEST=Mount a device and invoke by the command below, then confirm that the mount option changes.
dbus-send --system --dest=org.chromium.CrosDisks --print-reply /org/chromium/CrosDisks org.chromium.CrosDisks.Mount string:"$SOURCE_PATH" string:'' array:string:'ro','remount'
SOURCE_PATH is a full device name of the storage like "/sys/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.3/2-1.3:1.0/host9/target9:0:0/9:0:0:0/block/sdb/sdb1".

Change-Id: Ib495d9af7092c3d322456fa870799022532a602b
Reviewed-on: https://chromium-review.googlesource.com/390410
Commit-Ready: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Tested-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/b670bd19f986b0a30eb5ef84a73bbab175192ffe/cros-disks/mount_options.cc
[modify] https://crrev.com/b670bd19f986b0a30eb5ef84a73bbab175192ffe/cros-disks/mount_manager_unittest.cc
[modify] https://crrev.com/b670bd19f986b0a30eb5ef84a73bbab175192ffe/cros-disks/cros_disks_server.cc
[modify] https://crrev.com/b670bd19f986b0a30eb5ef84a73bbab175192ffe/cros-disks/mount_manager.cc
[modify] https://crrev.com/b670bd19f986b0a30eb5ef84a73bbab175192ffe/cros-disks/mount_manager.h
[modify] https://crrev.com/b670bd19f986b0a30eb5ef84a73bbab175192ffe/cros-disks/mount_options.h
[modify] https://crrev.com/b670bd19f986b0a30eb5ef84a73bbab175192ffe/cros-disks/cros_disks_server.h

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 24 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef

commit eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef
Author: yamaguchi <yamaguchi@chromium.org>
Date: Mon Oct 24 09:46:17 2016

Preserve the hardware read-only flag in Disk object.

The hardware read-only flag is obtained from device enumeration and
stored to Disk objects. Existing code overwrites that field when
applying read-only policy. However, that information will be needed when
we support remounting devices to switch between read-only and read-write
mode upon policy update.
After this change, Disk objects can distinguish whether a device is
mounted read-only because of a read-only hardware or because of the
mount option passed to Mount (for ExternalStorageReadOnly policy).

TEST=Run chromeos_unittest, unit_tests, and browser_tests.
BUG= 642247 , 655003 

Review-Url: https://codereview.chromium.org/2440443003
Cr-Commit-Position: refs/heads/master@{#427034}

[modify] https://crrev.com/eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef/chrome/browser/chromeos/extensions/file_manager/device_event_router_unittest.cc
[modify] https://crrev.com/eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef/chrome/browser/chromeos/extensions/file_manager/file_manager_private_apitest.cc
[modify] https://crrev.com/eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef/chrome/browser/chromeos/file_manager/volume_manager_unittest.cc
[modify] https://crrev.com/eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef/chromeos/disks/disk_mount_manager.cc
[modify] https://crrev.com/eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef/chromeos/disks/disk_mount_manager.h
[modify] https://crrev.com/eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef/chromeos/disks/disk_mount_manager_unittest.cc
[modify] https://crrev.com/eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef/chromeos/disks/mock_disk_mount_manager.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/de59ed6bed57372a9c00a7c65554abd47471f225

commit de59ed6bed57372a9c00a7c65554abd47471f225
Author: yamaguchi <yamaguchi@chromium.org>
Date: Tue Nov 08 10:07:10 2016

Add methods to remount removable devices in DiskMountManager.
The methods will be invoked from VolumeManager to flip the read-only mount option upon ExternalStorageReadOnly policy update.
Also allow to flip the read-only status of a Disk object from read-only to read-write, because it can happen when remounting a disk.

BUG= 642247 
TEST=chromeos_unittests --gtest_filter=DiskMountManagerTest.*

Review-Url: https://codereview.chromium.org/2451603002
Cr-Commit-Position: refs/heads/master@{#430564}

[modify] https://crrev.com/de59ed6bed57372a9c00a7c65554abd47471f225/chrome/browser/chromeos/extensions/file_manager/device_event_router.cc
[modify] https://crrev.com/de59ed6bed57372a9c00a7c65554abd47471f225/chrome/browser/chromeos/file_manager/fake_disk_mount_manager.cc
[modify] https://crrev.com/de59ed6bed57372a9c00a7c65554abd47471f225/chrome/browser/chromeos/file_manager/fake_disk_mount_manager.h
[modify] https://crrev.com/de59ed6bed57372a9c00a7c65554abd47471f225/chromeos/disks/disk_mount_manager.cc
[modify] https://crrev.com/de59ed6bed57372a9c00a7c65554abd47471f225/chromeos/disks/disk_mount_manager.h
[modify] https://crrev.com/de59ed6bed57372a9c00a7c65554abd47471f225/chromeos/disks/disk_mount_manager_unittest.cc
[modify] https://crrev.com/de59ed6bed57372a9c00a7c65554abd47471f225/chromeos/disks/mock_disk_mount_manager.cc
[modify] https://crrev.com/de59ed6bed57372a9c00a7c65554abd47471f225/chromeos/disks/mock_disk_mount_manager.h

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 10 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/28bce536b995429341b58d5bb6a826228bb6ef15

commit 28bce536b995429341b58d5bb6a826228bb6ef15
Author: yamaguchi <yamaguchi@chromium.org>
Date: Thu Nov 10 07:59:07 2016

Remount all removable storage devices when received update of
ExternalStorageReadOnly policy.

The end-to-end test can be done manually in this way:
1. Enter public session mode.
   See https://support.google.com/chrome/a/answer/3017014?hl=en
2. Plug a removable storage / MTP device.
3. Change the policy in Google admin and make sure it is reloaded at chrome://policy.
4. Check if the device is mounted RO/RW accordingly by right-clicking it in Files app. When the device is read-only, it the "format drive" menu will be grayed out.

This change depends on
https://chromium-review.googlesource.com/#/c/390410/
in chromeos. (which is already merged to trunk)

BUG= 642247 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
TEST=manual test as described above

Review-Url: https://codereview.chromium.org/2401963002
Cr-Commit-Position: refs/heads/master@{#431206}

[modify] https://crrev.com/28bce536b995429341b58d5bb6a826228bb6ef15/chrome/browser/chromeos/file_manager/volume_manager.cc
[modify] https://crrev.com/28bce536b995429341b58d5bb6a826228bb6ef15/chrome/browser/chromeos/file_manager/volume_manager.h
[modify] https://crrev.com/28bce536b995429341b58d5bb6a826228bb6ef15/chrome/browser/chromeos/file_manager/volume_manager_unittest.cc
[modify] https://crrev.com/28bce536b995429341b58d5bb6a826228bb6ef15/ui/file_manager/file_manager/background/js/volume_manager_impl.js

Comment 9 by fukino@chromium.org, Nov 11 2016

Labels: -M-55 M-56
Status: Fixed (was: Started)
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9057be33cf94f10c780e162c534b3e61526b195d

commit 9057be33cf94f10c780e162c534b3e61526b195d
Author: yamaguchi <yamaguchi@chromium.org>
Date: Tue Nov 15 05:02:52 2016

Flip the freature.dynamic_refresh flag of ExternalStorageReadOnly.
The policy is applied dynamically after the patch 2401963002:
https://codereview.chromium.org/2401963002

Also clarify the behavior when it's unset or false.

BUG=629945, 642247 

Review-Url: https://codereview.chromium.org/2398303003
Cr-Commit-Position: refs/heads/master@{#432105}

[modify] https://crrev.com/9057be33cf94f10c780e162c534b3e61526b195d/components/policy/resources/policy_templates.json

Status: Verified (was: Fixed)

Sign in to add a comment