linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Eric Biggers <ebiggers@google.com>
Subject: [PATCH 5.4 02/37] fscrypt: add fscrypt_symlink_getattr() for computing st_size
Date: Fri, 10 Sep 2021 14:30:05 +0200	[thread overview]
Message-ID: <20210910122917.228125255@linuxfoundation.org> (raw)
In-Reply-To: <20210910122917.149278545@linuxfoundation.org>

From: Eric Biggers <ebiggers@google.com>

commit d18760560593e5af921f51a8c9b64b6109d634c2 upstream.

Add a helper function fscrypt_symlink_getattr() which will be called
from the various filesystems' ->getattr() methods to read and decrypt
the target of encrypted symlinks in order to report the correct st_size.

Detailed explanation:

As required by POSIX and as documented in various man pages, st_size for
a symlink is supposed to be the length of the symlink target.
Unfortunately, st_size has always been wrong for encrypted symlinks
because st_size is populated from i_size from disk, which intentionally
contains the length of the encrypted symlink target.  That's slightly
greater than the length of the decrypted symlink target (which is the
symlink target that userspace usually sees), and usually won't match the
length of the no-key encoded symlink target either.

This hadn't been fixed yet because reporting the correct st_size would
require reading the symlink target from disk and decrypting or encoding
it, which historically has been considered too heavyweight to do in
->getattr().  Also historically, the wrong st_size had only broken a
test (LTP lstat03) and there were no known complaints from real users.
(This is probably because the st_size of symlinks isn't used too often,
and when it is, typically it's for a hint for what buffer size to pass
to readlink() -- which a slightly-too-large size still works for.)

However, a couple things have changed now.  First, there have recently
been complaints about the current behavior from real users:

- Breakage in rpmbuild:
  https://github.com/rpm-software-management/rpm/issues/1682
  https://github.com/google/fscrypt/issues/305

- Breakage in toybox cpio:
  https://www.mail-archive.com/toybox@lists.landley.net/msg07193.html

- Breakage in libgit2: https://issuetracker.google.com/issues/189629152
  (on Android public issue tracker, requires login)

Second, we now cache decrypted symlink targets in ->i_link.  Therefore,
taking the performance hit of reading and decrypting the symlink target
in ->getattr() wouldn't be as big a deal as it used to be, since usually
it will just save having to do the same thing later.

Also note that eCryptfs ended up having to read and decrypt symlink
targets in ->getattr() as well, to fix this same issue; see
commit 3a60a1686f0d ("eCryptfs: Decrypt symlink target for stat size").

So, let's just bite the bullet, and read and decrypt the symlink target
in ->getattr() in order to report the correct st_size.  Add a function
fscrypt_symlink_getattr() which the filesystems will call to do this.

(Alternatively, we could store the decrypted size of symlinks on-disk.
But there isn't a great place to do so, and encryption is meant to hide
the original size to some extent; that property would be lost.)

Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20210702065350.209646-2-ebiggers@kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/crypto/hooks.c       |   44 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fscrypt.h |    7 +++++++
 2 files changed, 51 insertions(+)

--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -305,3 +305,47 @@ err_kfree:
 	return ERR_PTR(err);
 }
 EXPORT_SYMBOL_GPL(fscrypt_get_symlink);
+
+/**
+ * fscrypt_symlink_getattr() - set the correct st_size for encrypted symlinks
+ * @path: the path for the encrypted symlink being queried
+ * @stat: the struct being filled with the symlink's attributes
+ *
+ * Override st_size of encrypted symlinks to be the length of the decrypted
+ * symlink target (or the no-key encoded symlink target, if the key is
+ * unavailable) rather than the length of the encrypted symlink target.  This is
+ * necessary for st_size to match the symlink target that userspace actually
+ * sees.  POSIX requires this, and some userspace programs depend on it.
+ *
+ * This requires reading the symlink target from disk if needed, setting up the
+ * inode's encryption key if possible, and then decrypting or encoding the
+ * symlink target.  This makes lstat() more heavyweight than is normally the
+ * case.  However, decrypted symlink targets will be cached in ->i_link, so
+ * usually the symlink won't have to be read and decrypted again later if/when
+ * it is actually followed, readlink() is called, or lstat() is called again.
+ *
+ * Return: 0 on success, -errno on failure
+ */
+int fscrypt_symlink_getattr(const struct path *path, struct kstat *stat)
+{
+	struct dentry *dentry = path->dentry;
+	struct inode *inode = d_inode(dentry);
+	const char *link;
+	DEFINE_DELAYED_CALL(done);
+
+	/*
+	 * To get the symlink target that userspace will see (whether it's the
+	 * decrypted target or the no-key encoded target), we can just get it in
+	 * the same way the VFS does during path resolution and readlink().
+	 */
+	link = READ_ONCE(inode->i_link);
+	if (!link) {
+		link = inode->i_op->get_link(dentry, inode, &done);
+		if (IS_ERR(link))
+			return PTR_ERR(link);
+	}
+	stat->size = strlen(link);
+	do_delayed_call(&done);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fscrypt_symlink_getattr);
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -298,6 +298,7 @@ extern int __fscrypt_encrypt_symlink(str
 extern const char *fscrypt_get_symlink(struct inode *inode, const void *caddr,
 				       unsigned int max_size,
 				       struct delayed_call *done);
+int fscrypt_symlink_getattr(const struct path *path, struct kstat *stat);
 static inline void fscrypt_set_ops(struct super_block *sb,
 				   const struct fscrypt_operations *s_cop)
 {
@@ -585,6 +586,12 @@ static inline const char *fscrypt_get_sy
 	return ERR_PTR(-EOPNOTSUPP);
 }
 
+static inline int fscrypt_symlink_getattr(const struct path *path,
+					  struct kstat *stat)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline void fscrypt_set_ops(struct super_block *sb,
 				   const struct fscrypt_operations *s_cop)
 {



  parent reply	other threads:[~2021-09-10 12:36 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-10 12:30 [PATCH 5.4 00/37] 5.4.145-rc1 review Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 01/37] ext4: fix race writing to an inline_data file while its xattrs are changing Greg Kroah-Hartman
2021-09-10 12:30 ` Greg Kroah-Hartman [this message]
2021-09-10 12:30 ` [PATCH 5.4 03/37] ext4: report correct st_size for encrypted symlinks Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 04/37] f2fs: " Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 05/37] ubifs: " Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 06/37] kthread: Fix PF_KTHREAD vs to_kthread() race Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 07/37] xtensa: fix kconfig unmet dependency warning for HAVE_FUTEX_CMPXCHG Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 08/37] gpu: ipu-v3: Fix i.MX IPU-v3 offset calculations for (semi)planar U/V formats Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 09/37] reset: reset-zynqmp: Fixed the argument data type Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 10/37] qed: Fix the VF msix vectors flow Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 11/37] net: macb: Add a NULL check on desc_ptp Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 12/37] qede: Fix memset corruption Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 13/37] perf/x86/intel/pt: Fix mask of num_address_ranges Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 14/37] perf/x86/amd/ibs: Work around erratum #1197 Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 15/37] perf/x86/amd/power: Assign pmu.module Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 16/37] cryptoloop: add a deprecation warning Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 17/37] ARM: 8918/2: only build return_address() if needed Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 18/37] ALSA: hda/realtek: Workaround for conflicting SSID on ASUS ROG Strix G17 Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 19/37] ALSA: pcm: fix divide error in snd_pcm_lib_ioctl Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 20/37] ARC: wireup clone3 syscall Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 21/37] media: stkwebcam: fix memory leak in stk_camera_probe Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 22/37] igmp: Add ip_mc_list lock in ip_check_mc_rcu Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 23/37] USB: serial: mos7720: improve OOM-handling in read_mos_reg() Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 24/37] ipv4/icmp: l3mdev: Perform icmp error route lookup on source device routing table (v2) Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 25/37] powerpc/boot: Delete unneeded .globl _zimage_start Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 26/37] net: ll_temac: Remove left-over debug message Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 27/37] mm/page_alloc: speed up the iteration of max_order Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 28/37] Revert "r8169: avoid link-up interrupt issue on RTL8106e if user enables ASPM" Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 29/37] x86/events/amd/iommu: Fix invalid Perf result due to IOMMU PMC power-gating Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 30/37] Revert "btrfs: compression: dont try to compress if we dont have enough pages" Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 31/37] ALSA: usb-audio: Add registration quirk for JBL Quantum 800 Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 32/37] usb: host: xhci-rcar: Dont reload firmware after the completion Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 33/37] usb: mtu3: use @mult for HS isoc or intr Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 34/37] usb: mtu3: fix the wrong HS mult value Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 35/37] xhci: fix unsafe memory usage in xhci tracing Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 36/37] x86/reboot: Limit Dell Optiplex 990 quirk to early BIOS versions Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.4 37/37] PCI: Call Max Payload Size-related fixup quirks early Greg Kroah-Hartman
2021-09-10 18:45 ` [PATCH 5.4 00/37] 5.4.145-rc1 review Florian Fainelli
2021-09-10 23:18 ` Shuah Khan
2021-09-11  6:11 ` Samuel Zou
2021-09-11 15:58 ` Sudip Mukherjee
2021-09-11 19:37 ` Guenter Roeck
2021-09-12  0:50 ` Daniel Díaz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210910122917.228125255@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=ebiggers@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).