ext4 crypto: Drop FS cache before invalidating the key |
|||||||||
Issue descriptionCurrently, Cryptohome's Platform::InvalidateDirCryptoKey() calls sync() and puts '3' to /proc/sys/vm/drop_caches after calling keyctl_invalidate(). (https://chromium.googlesource.com/chromiumos/platform2/+/master/cryptohome/platform.cc#1067) This is needed to clear plaintext data from the page cache. (see http://lists.openwall.net/linux-ext4/2015/12/11/2) However, according to tytso@google.com, the order should be reversed (i.e. sync() and echo 3 > /proc/sys/vm/drop_caches BEFORE calling keyctl_invalidate()) to avoid potential kernel crash. Also, we have to address race conditions like this: 1. sync() and echo 3 > /proc/sys/vm/drop_caches 2. <another process still writing data> 3. keyctl_invdalidate(). We should make sure no process is holding open FDs which belong to the encrypted directory.
,
Mar 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/c83ef6c9509fc0d6636e91ace2e35aa621d3d0c3 commit c83ef6c9509fc0d6636e91ace2e35aa621d3d0c3 Author: Ryo Hashimoto <hashimoto@google.com> Date: Sun Mar 05 12:40:31 2017 cryptohome: Fix dircrypto key invalidation logic Before this change, page caches were dropped after invalidating the key, and this can result in a kernel crash. After this change, dircrypto key is unlinked first, and then all page caches are dropped to invalidate the key. After that, we try to explicitly invalidate the key just to make sure. BUG= chromium:688220 TEST=Can add a user and login with direncryption enabled Change-Id: I50d33a28ca1f8d166bcfb660137139575b6dd88b Reviewed-on: https://chromium-review.googlesource.com/440747 Commit-Ready: Gwendal Grignou <gwendal@chromium.org> Tested-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-by: Gwendal Grignou <gwendal@chromium.org> [modify] https://crrev.com/c83ef6c9509fc0d6636e91ace2e35aa621d3d0c3/cryptohome/dircrypto_util.h [modify] https://crrev.com/c83ef6c9509fc0d6636e91ace2e35aa621d3d0c3/cryptohome/dircrypto_util.cc [modify] https://crrev.com/c83ef6c9509fc0d6636e91ace2e35aa621d3d0c3/cryptohome/platform.cc
,
Mar 6 2017
,
Mar 7 2017
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), bhthompson@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/cee8f639cb815bef2d9aaf8a05ddf299ef2067d7 commit cee8f639cb815bef2d9aaf8a05ddf299ef2067d7 Author: Ryo Hashimoto <hashimoto@google.com> Date: Wed Mar 08 08:50:47 2017 cryptohome: Fix dircrypto key invalidation logic Before this change, page caches were dropped after invalidating the key, and this can result in a kernel crash. After this change, dircrypto key is unlinked first, and then all page caches are dropped to invalidate the key. After that, we try to explicitly invalidate the key just to make sure. BUG= chromium:688220 TEST=Can add a user and login with direncryption enabled Change-Id: I50d33a28ca1f8d166bcfb660137139575b6dd88b Reviewed-on: https://chromium-review.googlesource.com/440747 Commit-Ready: Gwendal Grignou <gwendal@chromium.org> Tested-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-by: Gwendal Grignou <gwendal@chromium.org> (cherry picked from commit c83ef6c9509fc0d6636e91ace2e35aa621d3d0c3) Reviewed-on: https://chromium-review.googlesource.com/451167 Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org> Commit-Queue: Ryo Hashimoto <hashimoto@chromium.org> Tested-by: Ryo Hashimoto <hashimoto@chromium.org> [modify] https://crrev.com/cee8f639cb815bef2d9aaf8a05ddf299ef2067d7/cryptohome/dircrypto_util.h [modify] https://crrev.com/cee8f639cb815bef2d9aaf8a05ddf299ef2067d7/cryptohome/dircrypto_util.cc [modify] https://crrev.com/cee8f639cb815bef2d9aaf8a05ddf299ef2067d7/cryptohome/platform.cc
,
Mar 8 2017
,
Mar 10 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 13 2017
,
May 30 2017
,
Aug 1 2017
,
Jan 22 2018
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by hashimoto@chromium.org
, Feb 10 2017Owner: hashimoto@chromium.org
Status: Started (was: Assigned)