Message ID | 20200120060732.390362-1-ebiggers@kernel.org (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
Series | fscrypt: don't print name of busy file when removing key | expand |
On Sun, Jan 19, 2020 at 10:07:32PM -0800, Eric Biggers wrote: > From: Eric Biggers <ebiggers@google.com> > > When an encryption key can't be fully removed due to file(s) protected > by it still being in-use, we shouldn't really print the path to one of > these files to the kernel log, since parts of this path are likely to be > encrypted on-disk, and (depending on how the system is set up) the > confidentiality of this path might be lost by printing it to the log. > > This is a trade-off: a single file path often doesn't matter at all, > especially if it's a directory; the kernel log might still be protected > in some way; and I had originally hoped that any "inode(s) still busy" > bugs (which are security weaknesses in their own right) would be quickly > fixed and that to do so it would be super helpful to always know the > file path and not have to run 'find dir -inum $inum' after the fact. > > But in practice, these bugs can be hard to fix (e.g. due to asynchronous > process killing that is difficult to eliminate, for performance > reasons), and also not tied to specific files, so knowing a file path > doesn't necessarily help. > > So to be safe, for now let's just show the inode number, not the path. > If someone really wants to know a path they can use 'find -inum'. > > Fixes: b1c0ec3599f4 ("fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl") > Cc: <stable@vger.kernel.org> # v5.4+ > Signed-off-by: Eric Biggers <ebiggers@google.com> Applied to fscrypt.git#master for 5.6. - Eric
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c index 098ff2e0f0bb4..ab41b25d4fa1b 100644 --- a/fs/crypto/keyring.c +++ b/fs/crypto/keyring.c @@ -776,9 +776,6 @@ static int check_for_busy_inodes(struct super_block *sb, struct list_head *pos; size_t busy_count = 0; unsigned long ino; - struct dentry *dentry; - char _path[256]; - char *path = NULL; spin_lock(&mk->mk_decrypted_inodes_lock); @@ -797,22 +794,14 @@ static int check_for_busy_inodes(struct super_block *sb, struct fscrypt_info, ci_master_key_link)->ci_inode; ino = inode->i_ino; - dentry = d_find_alias(inode); } spin_unlock(&mk->mk_decrypted_inodes_lock); - if (dentry) { - path = dentry_path(dentry, _path, sizeof(_path)); - dput(dentry); - } - if (IS_ERR_OR_NULL(path)) - path = "(unknown)"; - fscrypt_warn(NULL, - "%s: %zu inode(s) still busy after removing key with %s %*phN, including ino %lu (%s)", + "%s: %zu inode(s) still busy after removing key with %s %*phN, including ino %lu", sb->s_id, busy_count, master_key_spec_type(&mk->mk_spec), master_key_spec_len(&mk->mk_spec), (u8 *)&mk->mk_spec.u, - ino, path); + ino); return -EBUSY; }