New issue
Advanced search Search tips

Issue 726930 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 15
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

ext4: patch 3.14 and 3.18 to be at same level than 4.4.70

Project Member Reported by gwendal@chromium.org, May 26 2017

Issue description

3.18 and 3.14 are behind in term of ext4 crypto patches:
Thanks to Eric Biggers, I can use the following lists to upstream/backport:

In Android 3.18:
https://android-review.googlesource.com/#/q/topic:3.18_ext4_crypto_fixes+(status:open+OR+status:merged) )

Between 4.4 and stable (4.4.70)
git log --pretty=oneline --grep='crypt' v4.4..v4.4.70 -- fs/ext4

[blocking b/38097492]
 
Labels: -Pri-3 Pri-1
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 14 2017

Labels: merge-merged-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/b9aadc2860764c560826ea4f57e03bd420e78703

commit b9aadc2860764c560826ea4f57e03bd420e78703
Author: Eric Biggers <ebiggers@google.com>
Date: Thu Sep 14 21:05:32 2017

UPSTREAM: fscrypto: add authorization check for setting encryption policy

commit 163ae1c6ad6299b19e22b4a35d5ab24a89791a98 upstream.

On an ext4 or f2fs filesystem with file encryption supported, a user
could set an encryption policy on any empty directory(*) to which they
had readonly access.  This is obviously problematic, since such a
directory might be owned by another user and the new encryption policy
would prevent that other user from creating files in their own directory
(for example).

Fix this by requiring inode_owner_or_capable() permission to set an
encryption policy.  This means that either the caller must own the file,
or the caller must have the capability CAP_FOWNER.

(*) Or also on any regular file, for f2fs v4.6 and later and ext4
    v4.8-rc1 and later; a separate bug fix is coming for that.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Apply changes to ext4 only, not f2fs.

BUG= chromium:726930 
TEST=build

(cherry picked from commit 8d693a2e67b5793ee58d106fded28902b7fd0f72)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: I92131acddce9e60b71af482976be2e81ae11b5a9
Reviewed-on: https://chromium-review.googlesource.com/549039
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Eric Biggers <ebiggers@google.com>

[modify] https://crrev.com/b9aadc2860764c560826ea4f57e03bd420e78703/fs/ext4/crypto_policy.c

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/370473d58aef278e0df602471ba48111b8c1d1a3

commit 370473d58aef278e0df602471ba48111b8c1d1a3
Author: Eric Biggers <ebiggers@google.com>
Date: Thu Sep 14 21:05:33 2017

UPSTREAM: fscrypto: require write access to mount to set encryption policy

commit ba63f23d69a3a10e7e527a02702023da68ef8a6d upstream.

Since setting an encryption policy requires writing metadata to the
filesystem, it should be guarded by mnt_want_write/mnt_drop_write.
Otherwise, a user could cause a write to a frozen or readonly
filesystem.  This was handled correctly by f2fs but not by ext4.  Make
fscrypt_process_policy() handle it rather than relying on the filesystem
to get it right.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: stable@vger.kernel.org # 4.1+; check fs/{ext4,f2fs}
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Acked-by: Jaegeuk Kim <jaegeuk@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

BUG= chromium:726930 
TEST=build

(cherry picked from commit bf63b9d429357bcb4857259874e36a44855f56ae)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: Ib20c95bcf8c66ad3138c800d7ea96b579e59c906
Reviewed-on: https://chromium-review.googlesource.com/549040
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Eric Biggers <ebiggers@google.com>

[modify] https://crrev.com/370473d58aef278e0df602471ba48111b8c1d1a3/fs/ext4/ioctl.c

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/807b9041d075e34a93942bd1450fcc2a7da1246b

commit 807b9041d075e34a93942bd1450fcc2a7da1246b
Author: Eric Whitney <enwlinux@gmail.com>
Date: Thu Sep 14 21:05:34 2017

UPSTREAM: ext4: enforce online defrag restriction for encrypted files

commit 14fbd4aa613bd5110556c281799ce36dc6f3ba97 upstream.

Online defragging of encrypted files is not currently implemented.
However, the move extent ioctl can still return successfully when
called.  For example, this occurs when xfstest ext4/020 is run on an
encrypted file system, resulting in a corrupted test file and a
corresponding test failure.

Until the proper functionality is implemented, fail the move extent
ioctl if either the original or donor file is encrypted.

Signed-off-by: Eric Whitney <enwlinux@gmail.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

BUG= chromium:726930 
TEST=build

(cherry picked from commit 76a8f17e0b850d5bb842097b0ee9c2e96af806a0)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: I046538b6a484ec38989d4e0df0c4986033477772
Reviewed-on: https://chromium-review.googlesource.com/549041
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Eric Biggers <ebiggers@google.com>

[modify] https://crrev.com/807b9041d075e34a93942bd1450fcc2a7da1246b/fs/ext4/move_extent.c

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/e59842d3d439f4445345a7843f1ed7a70e01304d

commit e59842d3d439f4445345a7843f1ed7a70e01304d
Author: Sergey Karamov <skaramov@google.com>
Date: Thu Sep 14 21:05:35 2017

BACKPORT: ext4: do not perform data journaling when data is encrypted

commit 73b92a2a5e97d17cc4d5c4fe9d724d3273fb6fd2 upstream.

Currently data journalling is incompatible with encryption: enabling both
at the same time has never been supported by design, and would result in
unpredictable behavior. However, users are not precluded from turning on
both features simultaneously. This change programmatically replaces data
journaling for encrypted regular files with ordered data journaling mode.

Background:
Journaling encrypted data has not been supported because it operates on
buffer heads of the page in the page cache. Namely, when the commit
happens, which could be up to five seconds after caching, the commit
thread uses the buffer heads attached to the page to copy the contents of
the page to the journal. With encryption, it would have been required to
keep the bounce buffer with ciphertext for up to the aforementioned five
seconds, since the page cache can only hold plaintext and could not be
used for journaling. Alternatively, it would be required to setup the
journal to initiate a callback at the commit time to perform deferred
encryption - in this case, not only would the data have to be written
twice, but it would also have to be encrypted twice. This level of
complexity was not justified for a mode that in practice is very rarely
used because of the overhead from the data journalling.

Solution:
If data=journaled has been set as a mount option for a filesystem, or if
journaling is enabled on a regular file, do not perform journaling if the
file is also encrypted, instead fall back to the data=ordered mode for the
file.

Rationale:
The intent is to allow seamless and proper filesystem operation when
journaling and encryption have both been enabled, and have these two
conflicting features gracefully resolved by the filesystem.

Fixes: 4461471107b7
Signed-off-by: Sergey Karamov <skaramov@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

merge conflict:
Use full macro instead ext4_has_feature_encrypt.

BUG= chromium:726930 
TEST=build

(cherry picked from commit 3460edfc70c21998f13662dceee9a26bf697d2c3)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: I31d4c45a2288a93ad8c09529cbfb8c43ae9c83cc
Reviewed-on: https://chromium-review.googlesource.com/549042
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Eric Biggers <ebiggers@google.com>

[modify] https://crrev.com/e59842d3d439f4445345a7843f1ed7a70e01304d/fs/ext4/super.c
[modify] https://crrev.com/e59842d3d439f4445345a7843f1ed7a70e01304d/fs/ext4/ext4_jbd2.h

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/68980a667119bead2ee6994545edd03397bdfcb9

commit 68980a667119bead2ee6994545edd03397bdfcb9
Author: Eric Biggers <ebiggers@google.com>
Date: Wed Oct 11 00:40:01 2017

UPSTREAM: fscrypt: fix renaming and linking special files

commit 42d97eb0ade31e1bc537d086842f5d6e766d9d51 upstream.

Attempting to link a device node, named pipe, or socket file into an
encrypted directory through rename(2) or link(2) always failed with
EPERM.  This happened because fscrypt_has_permitted_context() saw that
the file was unencrypted and forbid creating the link.  This behavior
was unexpected because such files are never encrypted; only regular
files, directories, and symlinks can be encrypted.

To fix this, make fscrypt_has_permitted_context() always return true on
special files.

This will be covered by a test in my encryption xfstests patchset.

Fixes: 9bd8212f981e ("ext4 crypto: add encryption policy and password salt support")
Signed-off-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Richard Weinberger <richard@nod.at>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Apply changes to ext4 only, not f2fs.

BUG= chromium:726930 
TEST=build

(cherry picked from commit fd74e8d258da9f9678da6bf88a0b02b2c1b71d0c)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: I43f57bbee5ad31728e3ce68de5dc727f46ec7630
Reviewed-on: https://chromium-review.googlesource.com/517247
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Eric Biggers <ebiggers@google.com>

[modify] https://crrev.com/68980a667119bead2ee6994545edd03397bdfcb9/fs/ext4/crypto_policy.c

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/41abc8aa40697c112309d79fd402990890e18511

commit 41abc8aa40697c112309d79fd402990890e18511
Author: Eric Biggers <ebiggers@google.com>
Date: Wed Oct 11 00:40:05 2017

UPSTREAM: fscrypt: fix context consistency check when key(s) unavailable

commit 272f98f6846277378e1758a49a49d7bf39343c02 upstream.

To mitigate some types of offline attacks, filesystem encryption is
designed to enforce that all files in an encrypted directory tree use
the same encryption policy (i.e. the same encryption context excluding
the nonce).  However, the fscrypt_has_permitted_context() function which
enforces this relies on comparing struct fscrypt_info's, which are only
available when we have the encryption keys.  This can cause two
incorrect behaviors:

1. If we have the parent directory's key but not the child's key, or
   vice versa, then fscrypt_has_permitted_context() returned false,
   causing applications to see EPERM or ENOKEY.  This is incorrect if
   the encryption contexts are in fact consistent.  Although we'd
   normally have either both keys or neither key in that case since the
   master_key_descriptors would be the same, this is not guaranteed
   because keys can be added or removed from keyrings at any time.

2. If we have neither the parent's key nor the child's key, then
   fscrypt_has_permitted_context() returned true, causing applications
   to see no error (or else an error for some other reason).  This is
   incorrect if the encryption contexts are in fact inconsistent, since
   in that case we should deny access.

To fix this, retrieve and compare the fscrypt_contexts if we are unable
to set up both fscrypt_infos.

While this slightly hurts performance when accessing an encrypted
directory tree without the key, this isn't a case we really need to be
optimizing for; access *with* the key is much more important.
Furthermore, the performance hit is barely noticeable given that we are
already retrieving the fscrypt_context and doing two keyring searches in
fscrypt_get_encryption_info().  If we ever actually wanted to optimize
this case we might start by caching the fscrypt_contexts.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Apply changes to ext4 only, not f2fs.

BUG= chromium:726930 
TEST=build

(cherry picked from commit 269d8211c400b42ff08cb1e047bd80e960c2705f)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: I190212b96f1840dd617f9c720e295b580be7f0ea
Reviewed-on: https://chromium-review.googlesource.com/517248
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Eric Biggers <ebiggers@google.com>

[modify] https://crrev.com/41abc8aa40697c112309d79fd402990890e18511/fs/ext4/crypto_policy.c

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/878bc76e2be2fdaa9df12fbc2b1765c0176b0c12

commit 878bc76e2be2fdaa9df12fbc2b1765c0176b0c12
Author: Eric Biggers <ebiggers@google.com>
Date: Wed Oct 11 00:40:02 2017

UPSTREAM: fscrypto: lock inode while setting encryption policy

commit 8906a8223ad4909b391c5628f7991ebceda30e52 upstream.

i_rwsem needs to be acquired while setting an encryption policy so that
concurrent calls to FS_IOC_SET_ENCRYPTION_POLICY are correctly
serialized (especially the ->get_context() + ->set_context() pair), and
so that new files cannot be created in the directory during or after the
->empty_dir() check.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: Richard Weinberger <richard@nod.at>
Cc: stable@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Apply changes to ext4 only, not f2fs.

BUG= chromium:726930 
TEST=build

(cherry picked from commit 3a19419c50c6ee386ca6d22a23acc2df51583d3d)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: If777dfbfb8beedb6b7f32145659cc6dabfc2c3b1
Reviewed-on: https://chromium-review.googlesource.com/549043
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Eric Biggers <ebiggers@google.com>

[modify] https://crrev.com/878bc76e2be2fdaa9df12fbc2b1765c0176b0c12/fs/ext4/ioctl.c

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/88e2cf81a7124331dea975f53b73d7a393e8d78d

commit 88e2cf81a7124331dea975f53b73d7a393e8d78d
Author: Guenter Roeck <groeck@chromium.org>
Date: Wed Oct 11 00:40:03 2017

CHROMIUM: ext4: Remove broken support for detecting keyring key revocation

Per upstream commit 1b53cf9815bb ("fscrypt: remove broken support for detecting
keyring key revocation"), keyring revocation detection is broken, and the
underlying code has been removed in v4.4.61. Remove local code to avoid build
breakage after the merge.

BUG= chromium:711780 , chromium:726930 
TEST=Build after merge

Change-Id: I2bac509fa87ece8ddff7cd01a24ac71134045236
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/479576
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
(cherry picked from commit f3e4b0af0b969fda15cb6e8294602fac15aa18d7)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/549044
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Eric Biggers <ebiggers@google.com>

[modify] https://crrev.com/88e2cf81a7124331dea975f53b73d7a393e8d78d/fs/ext4/crypto.c

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/f87c45e18417aef80bf482ae60c5acf0b3b78553

commit f87c45e18417aef80bf482ae60c5acf0b3b78553
Author: Eric Biggers <ebiggers@google.com>
Date: Wed Oct 11 00:40:04 2017

UPSTREAM: fscrypt: remove broken support for detecting keyring key revocation

commit 1b53cf9815bb4744958d41f3795d5d5a1d365e2d upstream.

Filesystem encryption ostensibly supported revoking a keyring key that
had been used to "unlock" encrypted files, causing those files to become
"locked" again.  This was, however, buggy for several reasons, the most
severe of which was that when key revocation happened to be detected for
an inode, its fscrypt_info was immediately freed, even while other
threads could be using it for encryption or decryption concurrently.
This could be exploited to crash the kernel or worse.

This patch fixes the use-after-free by removing the code which detects
the keyring key having been revoked, invalidated, or expired.  Instead,
an encrypted inode that is "unlocked" now simply remains unlocked until
it is evicted from memory.  Note that this is no worse than the case for
block device-level encryption, e.g. dm-crypt, and it still remains
possible for a privileged user to evict unused pages, inodes, and
dentries by running 'sync; echo 3 > /proc/sys/vm/drop_caches', or by
simply unmounting the filesystem.  In fact, one of those actions was
already needed anyway for key revocation to work even somewhat sanely.
This change is not expected to break any applications.

In the future I'd like to implement a real API for fscrypt key
revocation that interacts sanely with ongoing filesystem operations ---
waiting for existing operations to complete and blocking new operations,
and invalidating and sanitizing key material and plaintext from the VFS
caches.  But this is a hard problem, and for now this bug must be fixed.

This bug affected almost all versions of ext4, f2fs, and ubifs
encryption, and it was potentially reachable in any kernel configured
with encryption support (CONFIG_EXT4_ENCRYPTION=y,
CONFIG_EXT4_FS_ENCRYPTION=y, CONFIG_F2FS_FS_ENCRYPTION=y, or
CONFIG_UBIFS_FS_ENCRYPTION=y).  Note that older kernels did not use the
shared fs/crypto/ code, but due to the potential security implications
of this bug, it may still be worthwhile to backport this fix to them.

Fixes: b7236e21d55f ("ext4 crypto: reorganize how we store keys in the inode")
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Acked-by: Michael Halcrow <mhalcrow@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Apply changes to ext4 only, not f2fs.

BUG= chromium:726930 
TEST=build

(cherry picked from commit 7a5202190810dde1467718235c1f650fcf57592a)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: I364f3f73d34cdb899aba287f1617453f04bfb225
Reviewed-on: https://chromium-review.googlesource.com/549045
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Eric Biggers <ebiggers@google.com>

[modify] https://crrev.com/f87c45e18417aef80bf482ae60c5acf0b3b78553/fs/ext4/ext4_crypto.h
[modify] https://crrev.com/f87c45e18417aef80bf482ae60c5acf0b3b78553/fs/ext4/crypto_key.c
[modify] https://crrev.com/f87c45e18417aef80bf482ae60c5acf0b3b78553/fs/ext4/ext4.h

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 12 2017

Labels: merge-merged-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/f09bcd9ffbe29341e0895842e3e94e29ce0d0cbb

commit f09bcd9ffbe29341e0895842e3e94e29ce0d0cbb
Author: Eric Biggers <ebiggers@google.com>
Date: Thu Oct 12 03:45:05 2017

UPSTREAM: fscrypto: add authorization check for setting encryption policy

commit 163ae1c6ad6299b19e22b4a35d5ab24a89791a98 upstream.

On an ext4 or f2fs filesystem with file encryption supported, a user
could set an encryption policy on any empty directory(*) to which they
had readonly access.  This is obviously problematic, since such a
directory might be owned by another user and the new encryption policy
would prevent that other user from creating files in their own directory
(for example).

Fix this by requiring inode_owner_or_capable() permission to set an
encryption policy.  This means that either the caller must own the file,
or the caller must have the capability CAP_FOWNER.

(*) Or also on any regular file, for f2fs v4.6 and later and ext4
    v4.8-rc1 and later; a separate bug fix is coming for that.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Apply changes to ext4 only, not f2fs.

BUG= chromium:726930 
TEST=build boot samus, xfstests

(cherry picked from commit 8d693a2e67b5793ee58d106fded28902b7fd0f72)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: I92131acddce9e60b71af482976be2e81ae11b5a9
Reviewed-on: https://chromium-review.googlesource.com/549039
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Eric Biggers <ebiggers@google.com>
Reviewed-on: https://chromium-review.googlesource.com/696643
Commit-Ready: Sonny Rao <sonnyrao@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/f09bcd9ffbe29341e0895842e3e94e29ce0d0cbb/fs/ext4/crypto_policy.c

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/3c1f440d0af7abcbc53478030b97e48766aed1d3

commit 3c1f440d0af7abcbc53478030b97e48766aed1d3
Author: Eric Biggers <ebiggers@google.com>
Date: Thu Oct 26 04:58:53 2017

UPSTREAM: fscrypt: fix renaming and linking special files

commit 42d97eb0ade31e1bc537d086842f5d6e766d9d51 upstream.

Attempting to link a device node, named pipe, or socket file into an
encrypted directory through rename(2) or link(2) always failed with
EPERM.  This happened because fscrypt_has_permitted_context() saw that
the file was unencrypted and forbid creating the link.  This behavior
was unexpected because such files are never encrypted; only regular
files, directories, and symlinks can be encrypted.

To fix this, make fscrypt_has_permitted_context() always return true on
special files.

This will be covered by a test in my encryption xfstests patchset.

Fixes: 9bd8212f981e ("ext4 crypto: add encryption policy and password salt support")
Signed-off-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Richard Weinberger <richard@nod.at>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Apply changes to ext4 only, not f2fs.

BUG= chromium:726930 
TEST=Run xfstest on Samus

(cherry picked from commit fd74e8d258da9f9678da6bf88a0b02b2c1b71d0c)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: I0a02f285ba2d33df48653c4b8f402039f2b5889a
Reviewed-on: https://chromium-review.googlesource.com/734006
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/3c1f440d0af7abcbc53478030b97e48766aed1d3/fs/ext4/crypto_policy.c

Project Member

Comment 13 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/39bcbd946846b299fee8bff083ad215a218ee6bb

commit 39bcbd946846b299fee8bff083ad215a218ee6bb
Author: Eric Biggers <ebiggers@google.com>
Date: Thu Oct 26 04:58:54 2017

UPSTREAM: fscrypto: lock inode while setting encryption policy

commit 8906a8223ad4909b391c5628f7991ebceda30e52 upstream.

i_rwsem needs to be acquired while setting an encryption policy so that
concurrent calls to FS_IOC_SET_ENCRYPTION_POLICY are correctly
serialized (especially the ->get_context() + ->set_context() pair), and
so that new files cannot be created in the directory during or after the
->empty_dir() check.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: Richard Weinberger <richard@nod.at>
Cc: stable@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Apply changes to ext4 only, not f2fs.

BUG= chromium:726930 
TEST=run xfstest on Samus

(cherry picked from commit 3a19419c50c6ee386ca6d22a23acc2df51583d3d)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: Ibfde1ad607ea4f8f04f3fd916dd00c497af5c522
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/734007
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/39bcbd946846b299fee8bff083ad215a218ee6bb/fs/ext4/ioctl.c

Project Member

Comment 14 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/76a29c20fd562574c2970162e519becd7d6ddf0c

commit 76a29c20fd562574c2970162e519becd7d6ddf0c
Author: Guenter Roeck <groeck@chromium.org>
Date: Thu Oct 26 04:58:55 2017

CHROMIUM: ext4: Remove broken support for detecting keyring key revocation

Per upstream commit 1b53cf9815bb ("fscrypt: remove broken support for detecting
keyring key revocation"), keyring revocation detection is broken, and the
underlying code has been removed in v4.4.61. Remove local code to avoid build
breakage after the merge.

Apply to 3.14 as well, in ext4/crypto.c like in 3.18.

BUG= chromium:726930 , chromium:711780 
TEST=run xfstest on Samus

Change-Id: I2bac509fa87ece8ddff7cd01a24ac71134045236
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/479576
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
(cherry picked from commit f3e4b0af0b969fda15cb6e8294602fac15aa18d7)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/734008
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/76a29c20fd562574c2970162e519becd7d6ddf0c/fs/ext4/crypto.c

Project Member

Comment 15 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/8ca1772f34b12fa279ab3eab5c7b21fe421c1837

commit 8ca1772f34b12fa279ab3eab5c7b21fe421c1837
Author: Eric Biggers <ebiggers@google.com>
Date: Thu Oct 26 04:58:56 2017

UPSTREAM: fscrypt: remove broken support for detecting keyring key revocation

commit 1b53cf9815bb4744958d41f3795d5d5a1d365e2d upstream.

Filesystem encryption ostensibly supported revoking a keyring key that
had been used to "unlock" encrypted files, causing those files to become
"locked" again.  This was, however, buggy for several reasons, the most
severe of which was that when key revocation happened to be detected for
an inode, its fscrypt_info was immediately freed, even while other
threads could be using it for encryption or decryption concurrently.
This could be exploited to crash the kernel or worse.

This patch fixes the use-after-free by removing the code which detects
the keyring key having been revoked, invalidated, or expired.  Instead,
an encrypted inode that is "unlocked" now simply remains unlocked until
it is evicted from memory.  Note that this is no worse than the case for
block device-level encryption, e.g. dm-crypt, and it still remains
possible for a privileged user to evict unused pages, inodes, and
dentries by running 'sync; echo 3 > /proc/sys/vm/drop_caches', or by
simply unmounting the filesystem.  In fact, one of those actions was
already needed anyway for key revocation to work even somewhat sanely.
This change is not expected to break any applications.

In the future I'd like to implement a real API for fscrypt key
revocation that interacts sanely with ongoing filesystem operations ---
waiting for existing operations to complete and blocking new operations,
and invalidating and sanitizing key material and plaintext from the VFS
caches.  But this is a hard problem, and for now this bug must be fixed.

This bug affected almost all versions of ext4, f2fs, and ubifs
encryption, and it was potentially reachable in any kernel configured
with encryption support (CONFIG_EXT4_ENCRYPTION=y,
CONFIG_EXT4_FS_ENCRYPTION=y, CONFIG_F2FS_FS_ENCRYPTION=y, or
CONFIG_UBIFS_FS_ENCRYPTION=y).  Note that older kernels did not use the
shared fs/crypto/ code, but due to the potential security implications
of this bug, it may still be worthwhile to backport this fix to them.

Fixes: b7236e21d55f ("ext4 crypto: reorganize how we store keys in the inode")
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Acked-by: Michael Halcrow <mhalcrow@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Apply changes to ext4 only, not f2fs.

BUG= chromium:726930 
TEST=run xfstest on Samus

(cherry picked from commit 7a5202190810dde1467718235c1f650fcf57592a)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: Ifc9f129598d5fadb7b980e2076097d6574adda5d
Reviewed-on: https://chromium-review.googlesource.com/734009
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/8ca1772f34b12fa279ab3eab5c7b21fe421c1837/fs/ext4/ext4_crypto.h
[modify] https://crrev.com/8ca1772f34b12fa279ab3eab5c7b21fe421c1837/fs/ext4/crypto_key.c
[modify] https://crrev.com/8ca1772f34b12fa279ab3eab5c7b21fe421c1837/fs/ext4/ext4.h

Project Member

Comment 16 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/d913b19c11f9d127afc9fd4c8ad68ef45b4c4279

commit d913b19c11f9d127afc9fd4c8ad68ef45b4c4279
Author: Eric Biggers <ebiggers@google.com>
Date: Thu Oct 26 04:58:57 2017

UPSTREAM: fscrypt: fix context consistency check when key(s) unavailable

commit 272f98f6846277378e1758a49a49d7bf39343c02 upstream.

To mitigate some types of offline attacks, filesystem encryption is
designed to enforce that all files in an encrypted directory tree use
the same encryption policy (i.e. the same encryption context excluding
the nonce).  However, the fscrypt_has_permitted_context() function which
enforces this relies on comparing struct fscrypt_info's, which are only
available when we have the encryption keys.  This can cause two
incorrect behaviors:

1. If we have the parent directory's key but not the child's key, or
   vice versa, then fscrypt_has_permitted_context() returned false,
   causing applications to see EPERM or ENOKEY.  This is incorrect if
   the encryption contexts are in fact consistent.  Although we'd
   normally have either both keys or neither key in that case since the
   master_key_descriptors would be the same, this is not guaranteed
   because keys can be added or removed from keyrings at any time.

2. If we have neither the parent's key nor the child's key, then
   fscrypt_has_permitted_context() returned true, causing applications
   to see no error (or else an error for some other reason).  This is
   incorrect if the encryption contexts are in fact inconsistent, since
   in that case we should deny access.

To fix this, retrieve and compare the fscrypt_contexts if we are unable
to set up both fscrypt_infos.

While this slightly hurts performance when accessing an encrypted
directory tree without the key, this isn't a case we really need to be
optimizing for; access *with* the key is much more important.
Furthermore, the performance hit is barely noticeable given that we are
already retrieving the fscrypt_context and doing two keyring searches in
fscrypt_get_encryption_info().  If we ever actually wanted to optimize
this case we might start by caching the fscrypt_contexts.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

(cherry picked from commit 269d8211c400b42ff08cb1e047bd80e960c2705f)

Apply changes to ext4 only, not f2fs.

BUG= chromium:726930 
TEST=run xfstest on Samus

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: Ia8caa13d12b2a740f14ff5ff80857cbfccef90e8
Reviewed-on: https://chromium-review.googlesource.com/734010
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/d913b19c11f9d127afc9fd4c8ad68ef45b4c4279/fs/ext4/crypto_policy.c

Project Member

Comment 17 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/585579438a0de4fe1a214628a499233c8aaabf59

commit 585579438a0de4fe1a214628a499233c8aaabf59
Author: Rabin Vincent <rabinv@axis.com>
Date: Thu Oct 26 04:58:58 2017

UPSTREAM: block: protect iterate_bdevs() against concurrent close

[ Upstream commit af309226db916e2c6e08d3eba3fa5c34225200c4 ]

If a block device is closed while iterate_bdevs() is handling it, the
following NULL pointer dereference occurs because bdev->b_disk is NULL
in bdev_get_queue(), which is called from blk_get_backing_dev_info() (in
turn called by the mapping_cap_writeback_dirty() call in
__filemap_fdatawrite_range()):

 BUG: unable to handle kernel NULL pointer dereference at 0000000000000508
 IP: [<ffffffff81314790>] blk_get_backing_dev_info+0x10/0x20
 PGD 9e62067 PUD 9ee8067 PMD 0
 Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
 Modules linked in:
 CPU: 1 PID: 2422 Comm: sync Not tainted 4.5.0-rc7+ #400
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
 task: ffff880009f4d700 ti: ffff880009f5c000 task.ti: ffff880009f5c000
 RIP: 0010:[<ffffffff81314790>]  [<ffffffff81314790>] blk_get_backing_dev_info+0x10/0x20
 RSP: 0018:ffff880009f5fe68  EFLAGS: 00010246
 RAX: 0000000000000000 RBX: ffff88000ec17a38 RCX: ffffffff81a4e940
 RDX: 7fffffffffffffff RSI: 0000000000000000 RDI: ffff88000ec176c0
 RBP: ffff880009f5fe68 R08: 0000000000000000 R09: 0000000000000000
 R10: 0000000000000001 R11: 0000000000000000 R12: ffff88000ec17860
 R13: ffffffff811b25c0 R14: ffff88000ec178e0 R15: ffff88000ec17a38
 FS:  00007faee505d700(0000) GS:ffff88000fb00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
 CR2: 0000000000000508 CR3: 0000000009e8a000 CR4: 00000000000006e0
 Stack:
  ffff880009f5feb8 ffffffff8112e7f5 0000000000000000 7fffffffffffffff
  0000000000000000 0000000000000000 7fffffffffffffff 0000000000000001
  ffff88000ec178e0 ffff88000ec17860 ffff880009f5fec8 ffffffff8112e81f
 Call Trace:
  [<ffffffff8112e7f5>] __filemap_fdatawrite_range+0x85/0x90
  [<ffffffff8112e81f>] filemap_fdatawrite+0x1f/0x30
  [<ffffffff811b25d6>] fdatawrite_one_bdev+0x16/0x20
  [<ffffffff811bc402>] iterate_bdevs+0xf2/0x130
  [<ffffffff811b2763>] sys_sync+0x63/0x90
  [<ffffffff815d4272>] entry_SYSCALL_64_fastpath+0x12/0x76
 Code: 0f 1f 44 00 00 48 8b 87 f0 00 00 00 55 48 89 e5 <48> 8b 80 08 05 00 00 5d
 RIP  [<ffffffff81314790>] blk_get_backing_dev_info+0x10/0x20
  RSP <ffff880009f5fe68>
 CR2: 0000000000000508
 ---[ end trace 2487336ceb3de62d ]---

The crash is easily reproducible by running the following command, if an
msleep(100) is inserted before the call to func() in iterate_devs():

 while :; do head -c1 /dev/nullb0; done > /dev/null & while :; do sync; done

Fix it by holding the bd_mutex across the func() call and only calling
func() if the bdev is opened.

Cc: stable@vger.kernel.org
Fixes: 5c0d6b60a0ba ("vfs: Create function for iterating over block devices")
Reported-and-tested-by: Wei Fang <fangwei1@huawei.com>
Signed-off-by: Rabin Vincent <rabinv@axis.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@fb.com>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
(cherry picked from commit d0cfefba45916e4033fa54782f8965e51f355d55)

BUG= chromium:726930 
TEST=run xfstest on Samus

Change-Id: I59463d24d7e12471ee8a0365469877dc1fb9b248
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/734011
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/585579438a0de4fe1a214628a499233c8aaabf59/fs/block_dev.c

Project Member

Comment 18 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/2415ed040ded9c031a89d792955c1b5deb63e305

commit 2415ed040ded9c031a89d792955c1b5deb63e305
Author: NeilBrown <neilb@suse.com>
Date: Thu Oct 26 04:58:59 2017

UPSTREAM: block_dev: don't test bdev->bd_contains when it is not stable

[ Upstream commit bcc7f5b4bee8e327689a4d994022765855c807ff ]

bdev->bd_contains is not stable before calling __blkdev_get().
When __blkdev_get() is called on a parition with ->bd_openers == 0
it sets
  bdev->bd_contains = bdev;
which is not correct for a partition.
After a call to __blkdev_get() succeeds, ->bd_openers will be > 0
and then ->bd_contains is stable.

When FMODE_EXCL is used, blkdev_get() calls
   bd_start_claiming() ->  bd_prepare_to_claim() -> bd_may_claim()

This call happens before __blkdev_get() is called, so ->bd_contains
is not stable.  So bd_may_claim() cannot safely use ->bd_contains.
It currently tries to use it, and this can lead to a BUG_ON().

This happens when a whole device is already open with a bd_holder (in
use by dm in my particular example) and two threads race to open a
partition of that device for the first time, one opening with O_EXCL and
one without.

The thread that doesn't use O_EXCL gets through blkdev_get() to
__blkdev_get(), gains the ->bd_mutex, and sets bdev->bd_contains = bdev;

Immediately thereafter the other thread, using FMODE_EXCL, calls
bd_start_claiming() from blkdev_get().  This should fail because the
whole device has a holder, but because bdev->bd_contains == bdev
bd_may_claim() incorrectly reports success.
This thread continues and blocks on bd_mutex.

The first thread then sets bdev->bd_contains correctly and drops the mutex.
The thread using FMODE_EXCL then continues and when it calls bd_may_claim()
again in:
			BUG_ON(!bd_may_claim(bdev, whole, holder));
The BUG_ON fires.

Fix this by removing the dependency on ->bd_contains in
bd_may_claim().  As bd_may_claim() has direct access to the whole
device, it can simply test if the target bdev is the whole device.

Fixes: 6b4517a7913a ("block: implement bd_claiming and claiming block")
Cc: stable@vger.kernel.org (v2.6.35+)
Signed-off-by: NeilBrown <neilb@suse.com>
Signed-off-by: Jens Axboe <axboe@fb.com>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
(cherry picked from commit 9cdff4fe5279dc07351ecdd77a89db5ed6bba9af)

BUG= chromium:726930 
TEST=run xfstest on Samus

Change-Id: Id045af8db3f61425f14f09f24367ac1da15d0aca
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/734012
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/2415ed040ded9c031a89d792955c1b5deb63e305/fs/block_dev.c

Project Member

Comment 19 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/2a36623a3ce1dfd8bf8388a32051ef4539a68230

commit 2a36623a3ce1dfd8bf8388a32051ef4539a68230
Author: Andrey Ryabinin <aryabinin@virtuozzo.com>
Date: Thu Oct 26 04:59:01 2017

USPTREAM: fs/block_dev: always invalidate cleancache in invalidate_bdev()

commit a5f6a6a9c72eac38a7fadd1a038532bc8516337c upstream.

invalidate_bdev() calls cleancache_invalidate_inode() iff ->nrpages != 0
which doen't make any sense.

Make sure that invalidate_bdev() always calls cleancache_invalidate_inode()
regardless of mapping->nrpages value.

Fixes: c515e1fd361c ("mm/fs: add hooks to support cleancache")
Link: http://lkml.kernel.org/r/20170424164135.22350-3-aryabinin@virtuozzo.com
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Alexey Kuznetsov <kuznet@virtuozzo.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Nikolay Borisov <n.borisov.lkml@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Apply changes to ext4 only, not f2fs.

BUG= chromium:726930 
TEST=run xfstest on Samus

(cherry picked from commit 489bc9fed9735697b4387f8b8c9ec5b79e59f0b4)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: Ia8c1e060d659c9a4f4dfa3b6eda387792a22bf46
Reviewed-on: https://chromium-review.googlesource.com/734013
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/2a36623a3ce1dfd8bf8388a32051ef4539a68230/fs/block_dev.c

Project Member

Comment 20 by bugdroid1@chromium.org, Dec 19 2017

Labels: merge-merged-release-R63-10032.B-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/943805f2014ce925b188f78f74409bc6df0657a6

commit 943805f2014ce925b188f78f74409bc6df0657a6
Author: Eric Biggers <ebiggers@google.com>
Date: Tue Dec 19 18:16:37 2017

UPSTREAM: fscrypt: fix renaming and linking special files

commit 42d97eb0ade31e1bc537d086842f5d6e766d9d51 upstream.

Attempting to link a device node, named pipe, or socket file into an
encrypted directory through rename(2) or link(2) always failed with
EPERM.  This happened because fscrypt_has_permitted_context() saw that
the file was unencrypted and forbid creating the link.  This behavior
was unexpected because such files are never encrypted; only regular
files, directories, and symlinks can be encrypted.

To fix this, make fscrypt_has_permitted_context() always return true on
special files.

This will be covered by a test in my encryption xfstests patchset.

Fixes: 9bd8212f981e ("ext4 crypto: add encryption policy and password salt support")
Signed-off-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Richard Weinberger <richard@nod.at>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Apply changes to ext4 only, not f2fs.

BUG= chromium:726930 
TEST=Run xfstest on Samus

(cherry picked from commit fd74e8d258da9f9678da6bf88a0b02b2c1b71d0c)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: I0a02f285ba2d33df48653c4b8f402039f2b5889a
Reviewed-on: https://chromium-review.googlesource.com/734006
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 3c1f440d0af7abcbc53478030b97e48766aed1d3)
Reviewed-on: https://chromium-review.googlesource.com/834411
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
Commit-Queue: Gwendal Grignou <gwendal@chromium.org>

[modify] https://crrev.com/943805f2014ce925b188f78f74409bc6df0657a6/fs/ext4/crypto_policy.c

Project Member

Comment 21 by bugdroid1@chromium.org, Dec 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/fca8c1cf018b045448de3806b3e3fc6a071496ec

commit fca8c1cf018b045448de3806b3e3fc6a071496ec
Author: Eric Biggers <ebiggers@google.com>
Date: Tue Dec 19 18:16:40 2017

UPSTREAM: fscrypto: lock inode while setting encryption policy

commit 8906a8223ad4909b391c5628f7991ebceda30e52 upstream.

i_rwsem needs to be acquired while setting an encryption policy so that
concurrent calls to FS_IOC_SET_ENCRYPTION_POLICY are correctly
serialized (especially the ->get_context() + ->set_context() pair), and
so that new files cannot be created in the directory during or after the
->empty_dir() check.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: Richard Weinberger <richard@nod.at>
Cc: stable@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Apply changes to ext4 only, not f2fs.

BUG= chromium:726930 
TEST=run xfstest on Samus

(cherry picked from commit 3a19419c50c6ee386ca6d22a23acc2df51583d3d)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: Ibfde1ad607ea4f8f04f3fd916dd00c497af5c522
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/734007
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 39bcbd946846b299fee8bff083ad215a218ee6bb)
Reviewed-on: https://chromium-review.googlesource.com/834412

[modify] https://crrev.com/fca8c1cf018b045448de3806b3e3fc6a071496ec/fs/ext4/ioctl.c

Project Member

Comment 22 by bugdroid1@chromium.org, Dec 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/d43a64d2dc08997f6073beb94d89e8576e7e5506

commit d43a64d2dc08997f6073beb94d89e8576e7e5506
Author: Guenter Roeck <groeck@chromium.org>
Date: Tue Dec 19 18:16:44 2017

CHROMIUM: ext4: Remove broken support for detecting keyring key revocation

Per upstream commit 1b53cf9815bb ("fscrypt: remove broken support for detecting
keyring key revocation"), keyring revocation detection is broken, and the
underlying code has been removed in v4.4.61. Remove local code to avoid build
breakage after the merge.

Apply to 3.14 as well, in ext4/crypto.c like in 3.18.

BUG= chromium:726930 , chromium:711780 
TEST=run xfstest on Samus

Change-Id: I2bac509fa87ece8ddff7cd01a24ac71134045236
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/479576
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
(cherry picked from commit f3e4b0af0b969fda15cb6e8294602fac15aa18d7)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/734008
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 76a29c20fd562574c2970162e519becd7d6ddf0c)
Reviewed-on: https://chromium-review.googlesource.com/834413

[modify] https://crrev.com/d43a64d2dc08997f6073beb94d89e8576e7e5506/fs/ext4/crypto.c

Project Member

Comment 23 by bugdroid1@chromium.org, Dec 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/352aa2b7f00871dd611de95b6a28c5f1fbb98107

commit 352aa2b7f00871dd611de95b6a28c5f1fbb98107
Author: Eric Biggers <ebiggers@google.com>
Date: Tue Dec 19 18:16:47 2017

UPSTREAM: fscrypt: remove broken support for detecting keyring key revocation

commit 1b53cf9815bb4744958d41f3795d5d5a1d365e2d upstream.

Filesystem encryption ostensibly supported revoking a keyring key that
had been used to "unlock" encrypted files, causing those files to become
"locked" again.  This was, however, buggy for several reasons, the most
severe of which was that when key revocation happened to be detected for
an inode, its fscrypt_info was immediately freed, even while other
threads could be using it for encryption or decryption concurrently.
This could be exploited to crash the kernel or worse.

This patch fixes the use-after-free by removing the code which detects
the keyring key having been revoked, invalidated, or expired.  Instead,
an encrypted inode that is "unlocked" now simply remains unlocked until
it is evicted from memory.  Note that this is no worse than the case for
block device-level encryption, e.g. dm-crypt, and it still remains
possible for a privileged user to evict unused pages, inodes, and
dentries by running 'sync; echo 3 > /proc/sys/vm/drop_caches', or by
simply unmounting the filesystem.  In fact, one of those actions was
already needed anyway for key revocation to work even somewhat sanely.
This change is not expected to break any applications.

In the future I'd like to implement a real API for fscrypt key
revocation that interacts sanely with ongoing filesystem operations ---
waiting for existing operations to complete and blocking new operations,
and invalidating and sanitizing key material and plaintext from the VFS
caches.  But this is a hard problem, and for now this bug must be fixed.

This bug affected almost all versions of ext4, f2fs, and ubifs
encryption, and it was potentially reachable in any kernel configured
with encryption support (CONFIG_EXT4_ENCRYPTION=y,
CONFIG_EXT4_FS_ENCRYPTION=y, CONFIG_F2FS_FS_ENCRYPTION=y, or
CONFIG_UBIFS_FS_ENCRYPTION=y).  Note that older kernels did not use the
shared fs/crypto/ code, but due to the potential security implications
of this bug, it may still be worthwhile to backport this fix to them.

Fixes: b7236e21d55f ("ext4 crypto: reorganize how we store keys in the inode")
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Acked-by: Michael Halcrow <mhalcrow@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Apply changes to ext4 only, not f2fs.

BUG= chromium:726930 
TEST=run xfstest on Samus

(cherry picked from commit 7a5202190810dde1467718235c1f650fcf57592a)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: Ifc9f129598d5fadb7b980e2076097d6574adda5d
Reviewed-on: https://chromium-review.googlesource.com/734009
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 8ca1772f34b12fa279ab3eab5c7b21fe421c1837)
Reviewed-on: https://chromium-review.googlesource.com/834414
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
Commit-Queue: Gwendal Grignou <gwendal@chromium.org>

[modify] https://crrev.com/352aa2b7f00871dd611de95b6a28c5f1fbb98107/fs/ext4/ext4_crypto.h
[modify] https://crrev.com/352aa2b7f00871dd611de95b6a28c5f1fbb98107/fs/ext4/crypto_key.c
[modify] https://crrev.com/352aa2b7f00871dd611de95b6a28c5f1fbb98107/fs/ext4/ext4.h

Project Member

Comment 24 by bugdroid1@chromium.org, Dec 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/057aeffed0755e9b0e89c2ed0a0d95eac11ae402

commit 057aeffed0755e9b0e89c2ed0a0d95eac11ae402
Author: Eric Biggers <ebiggers@google.com>
Date: Tue Dec 19 18:16:50 2017

UPSTREAM: fscrypt: fix context consistency check when key(s) unavailable

commit 272f98f6846277378e1758a49a49d7bf39343c02 upstream.

To mitigate some types of offline attacks, filesystem encryption is
designed to enforce that all files in an encrypted directory tree use
the same encryption policy (i.e. the same encryption context excluding
the nonce).  However, the fscrypt_has_permitted_context() function which
enforces this relies on comparing struct fscrypt_info's, which are only
available when we have the encryption keys.  This can cause two
incorrect behaviors:

1. If we have the parent directory's key but not the child's key, or
   vice versa, then fscrypt_has_permitted_context() returned false,
   causing applications to see EPERM or ENOKEY.  This is incorrect if
   the encryption contexts are in fact consistent.  Although we'd
   normally have either both keys or neither key in that case since the
   master_key_descriptors would be the same, this is not guaranteed
   because keys can be added or removed from keyrings at any time.

2. If we have neither the parent's key nor the child's key, then
   fscrypt_has_permitted_context() returned true, causing applications
   to see no error (or else an error for some other reason).  This is
   incorrect if the encryption contexts are in fact inconsistent, since
   in that case we should deny access.

To fix this, retrieve and compare the fscrypt_contexts if we are unable
to set up both fscrypt_infos.

While this slightly hurts performance when accessing an encrypted
directory tree without the key, this isn't a case we really need to be
optimizing for; access *with* the key is much more important.
Furthermore, the performance hit is barely noticeable given that we are
already retrieving the fscrypt_context and doing two keyring searches in
fscrypt_get_encryption_info().  If we ever actually wanted to optimize
this case we might start by caching the fscrypt_contexts.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

(cherry picked from commit 269d8211c400b42ff08cb1e047bd80e960c2705f)

Apply changes to ext4 only, not f2fs.

BUG= chromium:726930 
TEST=run xfstest on Samus

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: Ia8caa13d12b2a740f14ff5ff80857cbfccef90e8
Reviewed-on: https://chromium-review.googlesource.com/734010
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit d913b19c11f9d127afc9fd4c8ad68ef45b4c4279)
Reviewed-on: https://chromium-review.googlesource.com/834415
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
Commit-Queue: Gwendal Grignou <gwendal@chromium.org>

[modify] https://crrev.com/057aeffed0755e9b0e89c2ed0a0d95eac11ae402/fs/ext4/crypto_policy.c

Project Member

Comment 25 by bugdroid1@chromium.org, Dec 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/7e6f4181567481d927518bec54818fac9a805179

commit 7e6f4181567481d927518bec54818fac9a805179
Author: Rabin Vincent <rabinv@axis.com>
Date: Tue Dec 19 18:16:53 2017

UPSTREAM: block: protect iterate_bdevs() against concurrent close

[ Upstream commit af309226db916e2c6e08d3eba3fa5c34225200c4 ]

If a block device is closed while iterate_bdevs() is handling it, the
following NULL pointer dereference occurs because bdev->b_disk is NULL
in bdev_get_queue(), which is called from blk_get_backing_dev_info() (in
turn called by the mapping_cap_writeback_dirty() call in
__filemap_fdatawrite_range()):

 BUG: unable to handle kernel NULL pointer dereference at 0000000000000508
 IP: [<ffffffff81314790>] blk_get_backing_dev_info+0x10/0x20
 PGD 9e62067 PUD 9ee8067 PMD 0
 Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
 Modules linked in:
 CPU: 1 PID: 2422 Comm: sync Not tainted 4.5.0-rc7+ #400
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
 task: ffff880009f4d700 ti: ffff880009f5c000 task.ti: ffff880009f5c000
 RIP: 0010:[<ffffffff81314790>]  [<ffffffff81314790>] blk_get_backing_dev_info+0x10/0x20
 RSP: 0018:ffff880009f5fe68  EFLAGS: 00010246
 RAX: 0000000000000000 RBX: ffff88000ec17a38 RCX: ffffffff81a4e940
 RDX: 7fffffffffffffff RSI: 0000000000000000 RDI: ffff88000ec176c0
 RBP: ffff880009f5fe68 R08: 0000000000000000 R09: 0000000000000000
 R10: 0000000000000001 R11: 0000000000000000 R12: ffff88000ec17860
 R13: ffffffff811b25c0 R14: ffff88000ec178e0 R15: ffff88000ec17a38
 FS:  00007faee505d700(0000) GS:ffff88000fb00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
 CR2: 0000000000000508 CR3: 0000000009e8a000 CR4: 00000000000006e0
 Stack:
  ffff880009f5feb8 ffffffff8112e7f5 0000000000000000 7fffffffffffffff
  0000000000000000 0000000000000000 7fffffffffffffff 0000000000000001
  ffff88000ec178e0 ffff88000ec17860 ffff880009f5fec8 ffffffff8112e81f
 Call Trace:
  [<ffffffff8112e7f5>] __filemap_fdatawrite_range+0x85/0x90
  [<ffffffff8112e81f>] filemap_fdatawrite+0x1f/0x30
  [<ffffffff811b25d6>] fdatawrite_one_bdev+0x16/0x20
  [<ffffffff811bc402>] iterate_bdevs+0xf2/0x130
  [<ffffffff811b2763>] sys_sync+0x63/0x90
  [<ffffffff815d4272>] entry_SYSCALL_64_fastpath+0x12/0x76
 Code: 0f 1f 44 00 00 48 8b 87 f0 00 00 00 55 48 89 e5 <48> 8b 80 08 05 00 00 5d
 RIP  [<ffffffff81314790>] blk_get_backing_dev_info+0x10/0x20
  RSP <ffff880009f5fe68>
 CR2: 0000000000000508
 ---[ end trace 2487336ceb3de62d ]---

The crash is easily reproducible by running the following command, if an
msleep(100) is inserted before the call to func() in iterate_devs():

 while :; do head -c1 /dev/nullb0; done > /dev/null & while :; do sync; done

Fix it by holding the bd_mutex across the func() call and only calling
func() if the bdev is opened.

Cc: stable@vger.kernel.org
Fixes: 5c0d6b60a0ba ("vfs: Create function for iterating over block devices")
Reported-and-tested-by: Wei Fang <fangwei1@huawei.com>
Signed-off-by: Rabin Vincent <rabinv@axis.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@fb.com>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
(cherry picked from commit d0cfefba45916e4033fa54782f8965e51f355d55)

BUG= chromium:726930 
TEST=run xfstest on Samus

Change-Id: I59463d24d7e12471ee8a0365469877dc1fb9b248
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/734011
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 585579438a0de4fe1a214628a499233c8aaabf59)
Reviewed-on: https://chromium-review.googlesource.com/834416

[modify] https://crrev.com/7e6f4181567481d927518bec54818fac9a805179/fs/block_dev.c

Project Member

Comment 26 by bugdroid1@chromium.org, Dec 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/10f68139b62d1a99e6b1ed90bdce07733f631469

commit 10f68139b62d1a99e6b1ed90bdce07733f631469
Author: NeilBrown <neilb@suse.com>
Date: Tue Dec 19 18:16:56 2017

UPSTREAM: block_dev: don't test bdev->bd_contains when it is not stable

[ Upstream commit bcc7f5b4bee8e327689a4d994022765855c807ff ]

bdev->bd_contains is not stable before calling __blkdev_get().
When __blkdev_get() is called on a parition with ->bd_openers == 0
it sets
  bdev->bd_contains = bdev;
which is not correct for a partition.
After a call to __blkdev_get() succeeds, ->bd_openers will be > 0
and then ->bd_contains is stable.

When FMODE_EXCL is used, blkdev_get() calls
   bd_start_claiming() ->  bd_prepare_to_claim() -> bd_may_claim()

This call happens before __blkdev_get() is called, so ->bd_contains
is not stable.  So bd_may_claim() cannot safely use ->bd_contains.
It currently tries to use it, and this can lead to a BUG_ON().

This happens when a whole device is already open with a bd_holder (in
use by dm in my particular example) and two threads race to open a
partition of that device for the first time, one opening with O_EXCL and
one without.

The thread that doesn't use O_EXCL gets through blkdev_get() to
__blkdev_get(), gains the ->bd_mutex, and sets bdev->bd_contains = bdev;

Immediately thereafter the other thread, using FMODE_EXCL, calls
bd_start_claiming() from blkdev_get().  This should fail because the
whole device has a holder, but because bdev->bd_contains == bdev
bd_may_claim() incorrectly reports success.
This thread continues and blocks on bd_mutex.

The first thread then sets bdev->bd_contains correctly and drops the mutex.
The thread using FMODE_EXCL then continues and when it calls bd_may_claim()
again in:
			BUG_ON(!bd_may_claim(bdev, whole, holder));
The BUG_ON fires.

Fix this by removing the dependency on ->bd_contains in
bd_may_claim().  As bd_may_claim() has direct access to the whole
device, it can simply test if the target bdev is the whole device.

Fixes: 6b4517a7913a ("block: implement bd_claiming and claiming block")
Cc: stable@vger.kernel.org (v2.6.35+)
Signed-off-by: NeilBrown <neilb@suse.com>
Signed-off-by: Jens Axboe <axboe@fb.com>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
(cherry picked from commit 9cdff4fe5279dc07351ecdd77a89db5ed6bba9af)

BUG= chromium:726930 
TEST=run xfstest on Samus

Change-Id: Id045af8db3f61425f14f09f24367ac1da15d0aca
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/734012
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 2415ed040ded9c031a89d792955c1b5deb63e305)
Reviewed-on: https://chromium-review.googlesource.com/834417

[modify] https://crrev.com/10f68139b62d1a99e6b1ed90bdce07733f631469/fs/block_dev.c

Project Member

Comment 27 by bugdroid1@chromium.org, Dec 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/1552fd495d99e6f5d842f3eb3bc6decc0a0f852f

commit 1552fd495d99e6f5d842f3eb3bc6decc0a0f852f
Author: Andrey Ryabinin <aryabinin@virtuozzo.com>
Date: Tue Dec 19 18:16:59 2017

USPTREAM: fs/block_dev: always invalidate cleancache in invalidate_bdev()

commit a5f6a6a9c72eac38a7fadd1a038532bc8516337c upstream.

invalidate_bdev() calls cleancache_invalidate_inode() iff ->nrpages != 0
which doen't make any sense.

Make sure that invalidate_bdev() always calls cleancache_invalidate_inode()
regardless of mapping->nrpages value.

Fixes: c515e1fd361c ("mm/fs: add hooks to support cleancache")
Link: http://lkml.kernel.org/r/20170424164135.22350-3-aryabinin@virtuozzo.com
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Alexey Kuznetsov <kuznet@virtuozzo.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Nikolay Borisov <n.borisov.lkml@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Apply changes to ext4 only, not f2fs.

BUG= chromium:726930 
TEST=run xfstest on Samus

(cherry picked from commit 489bc9fed9735697b4387f8b8c9ec5b79e59f0b4)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>

Change-Id: Ia8c1e060d659c9a4f4dfa3b6eda387792a22bf46
Reviewed-on: https://chromium-review.googlesource.com/734013
Commit-Ready: Gwendal Grignou <gwendal@chromium.org>
Tested-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 2a36623a3ce1dfd8bf8388a32051ef4539a68230)
Reviewed-on: https://chromium-review.googlesource.com/834418
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
Commit-Queue: Gwendal Grignou <gwendal@chromium.org>

[modify] https://crrev.com/1552fd495d99e6f5d842f3eb3bc6decc0a0f852f/fs/block_dev.c

Status: Fixed (was: Started)

Sign in to add a comment