imageloader avoids copy at RegisterComponent&LoadComponent |
|||||||||
Issue descriptionimageloader currently makes a copy of component at /var/lib/imageloader/<component>/<version> which takes up more space than required. It frees up half space if imageloader can avoid copy at RegisterComponent and LoadComponent in place. Register a component: * Avoid calling imageloader RegisterComponent * OnCustomInstall does nothing which requires no callback and current threading model works fine. Load a component: * uses imageloader API LoadComponentAtPath to load a component directly in-place. * cache component install path from browser_process_platform_part_chromeos (from ComponentReady) so that LoadComponentAtPath can use. benefits: 1. fewer space requirement: component updater & imageloader shares the same copy. 2. no threading issue any more since time consuming copying no longer happens in OnCustomInstall (which has to be async).
,
Nov 29 2017
,
Nov 29 2017
,
Nov 29 2017
,
Nov 29 2017
,
Nov 29 2017
can you please review this?
,
Nov 29 2017
,
Nov 29 2017
Has someone on the security side reviewed this? How will the paths be sanitized?
,
Nov 29 2017
The path is provided by ComponentReady which is a callback called by component updater after a component is installed. LoadComponent verifies the integrity of component. Actually, component at the path that ComponentReady provides can always be copied by imageloader successfully and LoadComponent mount the copy with all the verification. In this case, the path provided by ComponentReady should be safe. What do you think kerrnel@?
,
Nov 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/system_api/+/7092a3102eae259d6ccca85a8f68d0ebf78394d2 commit 7092a3102eae259d6ccca85a8f68d0ebf78394d2 Author: Xiaochu Liu <xiaochu@chromium.org> Date: Thu Nov 30 09:13:16 2017 Add constant to imageloader dbus API Add LoadComponentAtPath to imageloader dbus API. BUG= chromium:780575 TEST=None Change-Id: I468586284afd077ce5f7b0c45a9a39eb68039013 Reviewed-on: https://chromium-review.googlesource.com/797359 Commit-Ready: Xiaochu Liu <xiaochu@chromium.org> Tested-by: Xiaochu Liu <xiaochu@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> [modify] https://crrev.com/7092a3102eae259d6ccca85a8f68d0ebf78394d2/dbus/service_constants.h
,
Nov 30 2017
We don't sanitize the paths because ImageLoader will only load anything signed by Google, and then it's verified with dm-verity.
,
Nov 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9d24ee15befd89c494b9e502cf49d25a662831ee commit 9d24ee15befd89c494b9e502cf49d25a662831ee Author: Xiaochu Liu <xiaochu@chromium.org> Date: Thu Nov 30 20:53:05 2017 Roll src/third_party/cros_system_api/ 9b6a22544..7092a3102 (3 commits) https://chromium.googlesource.com/chromiumos/platform/system_api.git/+log/9b6a22544458..7092a3102eae $ git log 9b6a22544..7092a3102 --date=short --no-merges --format='%ad %ae %s' 2017-11-29 xiaochu Add constant to imageloader dbus API 2017-11-29 derat system_api: Document potential races in suspend readiness. 2017-11-27 allenvic smbprovider: Add ReadDirectoryOptions and GetMetadataEntryOptions Created with: roll-dep src/third_party/cros_system_api Bug= chromium:780575 TEST=None Change-Id: I0f633691030275ebf8d3e398efc9b9a568a69db9 Reviewed-on: https://chromium-review.googlesource.com/801211 Reviewed-by: Dan Erat <derat@chromium.org> Commit-Queue: Xiaochu Liu <xiaochu@chromium.org> Cr-Commit-Position: refs/heads/master@{#520673} [modify] https://crrev.com/9d24ee15befd89c494b9e502cf49d25a662831ee/DEPS
,
Dec 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a8fb3d1f496e54e2abca7fbde01257434957552f commit a8fb3d1f496e54e2abca7fbde01257434957552f Author: Xiaochu Liu <xiaochu@chromium.org> Date: Tue Dec 05 06:07:01 2017 avoid copy in component registration Skip imageloader RegisterComponent (component copy) in OnCustomInstall and loads the copy in chrome. Additions: Add LoadComponentAtPath in ImageLoaderClient. Save the (compatible) component path in browser_process. BUG= chromium:780575 TEST=unittest; install/load component successfully. Change-Id: Ic14f7d33ee6c8971dd6326f39fda546c94959e14 Reviewed-on: https://chromium-review.googlesource.com/801535 Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> Reviewed-by: Joshua Pawlicki <waffles@chromium.org> Commit-Queue: Xiaochu Liu <xiaochu@chromium.org> Cr-Commit-Position: refs/heads/master@{#521622} [modify] https://crrev.com/a8fb3d1f496e54e2abca7fbde01257434957552f/chrome/browser/browser_process_platform_part_chromeos.cc [modify] https://crrev.com/a8fb3d1f496e54e2abca7fbde01257434957552f/chrome/browser/browser_process_platform_part_chromeos.h [modify] https://crrev.com/a8fb3d1f496e54e2abca7fbde01257434957552f/chrome/browser/component_updater/cros_component_installer.cc [modify] https://crrev.com/a8fb3d1f496e54e2abca7fbde01257434957552f/chrome/browser/component_updater/cros_component_installer_unittest.cc [modify] https://crrev.com/a8fb3d1f496e54e2abca7fbde01257434957552f/chromeos/dbus/fake_image_loader_client.cc [modify] https://crrev.com/a8fb3d1f496e54e2abca7fbde01257434957552f/chromeos/dbus/fake_image_loader_client.h [modify] https://crrev.com/a8fb3d1f496e54e2abca7fbde01257434957552f/chromeos/dbus/image_loader_client.cc [modify] https://crrev.com/a8fb3d1f496e54e2abca7fbde01257434957552f/chromeos/dbus/image_loader_client.h
,
Dec 5 2017
thanks all!
,
Jul 30
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by xiaochu@chromium.org
, Nov 1 2017