[PATCH] fscrypt: allow filesystems to disable symlink target caching

Alex Markuze posted 1 patch 1 week ago
fs/ceph/crypto.c        |  1 +
fs/crypto/hooks.c       | 37 ++++++++++++++++++++++---------------
include/linux/fscrypt.h |  7 +++++++
3 files changed, 30 insertions(+), 15 deletions(-)
[PATCH] fscrypt: allow filesystems to disable symlink target caching
Posted by Alex Markuze 1 week ago
fscrypt stores decrypted symlink targets in inode->i_link. The VFS can
use this field without calling the filesystem's ->get_link() callback.
The target can remain valid after the access policy changes.

The CephFS subvolume quarantine series adds access checks to
->get_link(). Quarantine can revoke access after a target is cached. The
VFS cache can bypass these checks.

The no_symlink_cache option in fscrypt_operations gives filesystems
control of this cache. It applies during target creation, lookup and
getattr. With this option set, the helpers do not read or update
->i_link. The filesystem callback can then revalidate access.

Uncached targets use the existing delayed-free callback. Each uncached
lookup allocates a temporary buffer and decrypts or encodes the target.
This lets Ceph reuse fscrypt's target validation and buffer ownership.

The default setting keeps the current cache behavior for other
filesystems. Ceph selects the option so cached targets cannot bypass its
->get_link() checks.

Signed-off-by: Alex Markuze <amarkuze@redhat.com>
---
 fs/ceph/crypto.c        |  1 +
 fs/crypto/hooks.c       | 37 ++++++++++++++++++++++---------------
 include/linux/fscrypt.h |  7 +++++++
 3 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index 9e0ebb071840..de786a0e626c 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -92,6 +92,7 @@ static struct fscrypt_operations ceph_fscrypt_ops = {
 	.inode_info_offs	= (int)offsetof(struct ceph_inode_info, i_crypt_info) -
 				  (int)offsetof(struct ceph_inode_info, netfs.inode),
 	.needs_bounce_pages	= 1,
+	.no_symlink_cache	= 1,
 	.get_context		= ceph_crypt_get_context,
 	.set_context		= ceph_crypt_set_context,
 	.get_dummy_policy	= ceph_get_dummy_policy,
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index a7a8a3f581a0..e1eb0fce7948 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -321,11 +321,13 @@ int __fscrypt_encrypt_symlink(struct inode *inode, const char *target,
 	 */
 	sd->encrypted_path[ciphertext_len] = '\0';
 
-	/* Cache the plaintext symlink target for later use by get_link() */
-	err = -ENOMEM;
-	inode->i_link = kmemdup(target, len + 1, GFP_NOFS);
-	if (!inode->i_link)
-		goto err_free_sd;
+	/* Cache the plaintext symlink target if the filesystem allows it. */
+	if (!inode->i_sb->s_cop->no_symlink_cache) {
+		err = -ENOMEM;
+		inode->i_link = kmemdup(target, len + 1, GFP_NOFS);
+		if (!inode->i_link)
+			goto err_free_sd;
+	}
 
 	if (!disk_link->name)
 		disk_link->name = (unsigned char *)sd;
@@ -358,17 +360,21 @@ const char *fscrypt_get_symlink(struct inode *inode, const void *caddr,
 {
 	const struct fscrypt_symlink_data *sd;
 	struct fscrypt_str cstr, pstr;
-	bool has_key;
+	bool has_key, cache;
 	int err;
 
 	/* This is for encrypted symlinks only */
 	if (WARN_ON_ONCE(!IS_ENCRYPTED(inode)))
 		return ERR_PTR(-EINVAL);
 
+	cache = !inode->i_sb->s_cop->no_symlink_cache;
+
 	/* If the decrypted target is already cached, just return it. */
-	pstr.name = READ_ONCE(inode->i_link);
-	if (pstr.name)
-		return pstr.name;
+	if (cache) {
+		pstr.name = READ_ONCE(inode->i_link);
+		if (pstr.name)
+			return pstr.name;
+	}
 
 	/*
 	 * Try to set up the symlink's encryption key, but we can continue
@@ -411,12 +417,12 @@ const char *fscrypt_get_symlink(struct inode *inode, const void *caddr,
 	pstr.name[pstr.len] = '\0';
 
 	/*
-	 * Cache decrypted symlink targets in i_link for later use.  Don't cache
-	 * symlink targets encoded without the key, since those become outdated
-	 * once the key is added.  This pairs with the READ_ONCE() above and in
-	 * the VFS path lookup code.
+	 * Cache decrypted symlink targets in i_link if the filesystem allows it.
+	 * Don't cache symlink targets encoded without the key, since those
+	 * become outdated once the key is added.  This pairs with the
+	 * READ_ONCE() above and in the VFS path lookup code.
 	 */
-	if (!has_key ||
+	if (!cache || !has_key ||
 	    cmpxchg_release(&inode->i_link, NULL, pstr.name) != NULL)
 		set_delayed_call(done, kfree_link, pstr.name);
 
@@ -460,7 +466,8 @@ int fscrypt_symlink_getattr(const struct path *path, struct kstat *stat)
 	 * 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);
+	link = inode->i_sb->s_cop->no_symlink_cache ?
+		NULL : READ_ONCE(inode->i_link);
 	if (!link) {
 		link = inode->i_op->get_link(dentry, inode, &done);
 		if (IS_ERR(link))
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 28f2108e29f3..faeff5812885 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -107,6 +107,13 @@ struct fscrypt_operations {
 	 */
 	unsigned int supports_subblock_data_units : 1;
 
+	/*
+	 * If set, don't cache decrypted symlink targets in ->i_link.  This
+	 * ensures that the VFS calls ->get_link() for every access, allowing
+	 * the filesystem to revalidate the symlink before returning its target.
+	 */
+	unsigned int no_symlink_cache : 1;
+
 	/*
 	 * This field exists only for backwards compatibility reasons and should
 	 * only be set by the filesystems that are setting it already.  It

base-commit: ed0502cb11f84fff29f374238e526ae94d9bdfde
-- 
2.34.1
Re: [PATCH] fscrypt: allow filesystems to disable symlink target caching
Posted by Eric Biggers 1 week ago
On Thu, Oct 15, 2026 at 05:46:24PM +0000, Alex Markuze wrote:
> fscrypt stores decrypted symlink targets in inode->i_link. The VFS can
> use this field without calling the filesystem's ->get_link() callback.
> The target can remain valid after the access policy changes.
> 
> The CephFS subvolume quarantine series adds access checks to
> ->get_link(). Quarantine can revoke access after a target is cached. The
> VFS cache can bypass these checks.
> 
> The no_symlink_cache option in fscrypt_operations gives filesystems
> control of this cache. It applies during target creation, lookup and
> getattr. With this option set, the helpers do not read or update
> ->i_link. The filesystem callback can then revalidate access.
> 
> Uncached targets use the existing delayed-free callback. Each uncached
> lookup allocates a temporary buffer and decrypts or encodes the target.
> This lets Ceph reuse fscrypt's target validation and buffer ownership.
> 
> The default setting keeps the current cache behavior for other
> filesystems. Ceph selects the option so cached targets cannot bypass its
> ->get_link() checks.
> 
> Signed-off-by: Alex Markuze <amarkuze@redhat.com>

This sounds like a bug fix?  If so, which commit is it fixing?

- Eric
Re: [PATCH] fscrypt: allow filesystems to disable symlink target caching
Posted by Alex Markuze 1 week ago
Hi Eric,

This supports the concurrent CephFS subvolume quarantine series.
There’s no Fixes: tag since this isn't addressing an existing upstream
regression.

The quarantine series adds checks to Ceph's ->get_link(). However, if
an encrypted symlink target was cached prior to quarantine, the VFS
bypasses that callback entirely when following or reading it. While
the target data is still valid, access now needs to be denied.

This patch adds an opt-out for symlink caching and enables it for
Ceph, handling the encrypted-symlink case for the concurrent series
without affecting other filesystems.

On Thu, Oct 15, 2026 at 9:35 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Thu, Oct 15, 2026 at 05:46:24PM +0000, Alex Markuze wrote:
> > fscrypt stores decrypted symlink targets in inode->i_link. The VFS can
> > use this field without calling the filesystem's ->get_link() callback.
> > The target can remain valid after the access policy changes.
> >
> > The CephFS subvolume quarantine series adds access checks to
> > ->get_link(). Quarantine can revoke access after a target is cached. The
> > VFS cache can bypass these checks.
> >
> > The no_symlink_cache option in fscrypt_operations gives filesystems
> > control of this cache. It applies during target creation, lookup and
> > getattr. With this option set, the helpers do not read or update
> > ->i_link. The filesystem callback can then revalidate access.
> >
> > Uncached targets use the existing delayed-free callback. Each uncached
> > lookup allocates a temporary buffer and decrypts or encodes the target.
> > This lets Ceph reuse fscrypt's target validation and buffer ownership.
> >
> > The default setting keeps the current cache behavior for other
> > filesystems. Ceph selects the option so cached targets cannot bypass its
> > ->get_link() checks.
> >
> > Signed-off-by: Alex Markuze <amarkuze@redhat.com>
>
> This sounds like a bug fix?  If so, which commit is it fixing?
>
> - Eric
>
Re: [PATCH] fscrypt: allow filesystems to disable symlink target caching
Posted by Eric Biggers 1 week ago
On Thu, Oct 15, 2026 at 09:52:11PM +0300, Alex Markuze wrote:
> Hi Eric,
> 
> This supports the concurrent CephFS subvolume quarantine series.
> There’s no Fixes: tag since this isn't addressing an existing upstream
> regression.
> 
> The quarantine series adds checks to Ceph's ->get_link(). However, if
> an encrypted symlink target was cached prior to quarantine, the VFS
> bypasses that callback entirely when following or reading it. While
> the target data is still valid, access now needs to be denied.
> 
> This patch adds an opt-out for symlink caching and enables it for
> Ceph, handling the encrypted-symlink case for the concurrent series
> without affecting other filesystems.
> 

That series wasn't sent to me.  Is it
https://lore.kernel.org/r/cover.1792074165.git.amarkuze@redhat.com/ ?
It would make more sense to add this patch to that series so that they
can be reviewed together.  I can potentially apply a prerequisite patch
by itself, but we'd first need to know that the rest has been reviewed
and will be accepted too, otherwise it will be unused code.

- Eric
Re: [PATCH] fscrypt: allow filesystems to disable symlink target caching
Posted by Alex Markuze 1 week ago
I didn't want to conflate fscrypt patches with Ceph features; arguably
two distinct patchsets.

I will CC you folks when this is acked then.

On Thu, Oct 15, 2026 at 10:38 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Thu, Oct 15, 2026 at 09:52:11PM +0300, Alex Markuze wrote:
> > Hi Eric,
> >
> > This supports the concurrent CephFS subvolume quarantine series.
> > There’s no Fixes: tag since this isn't addressing an existing upstream
> > regression.
> >
> > The quarantine series adds checks to Ceph's ->get_link(). However, if
> > an encrypted symlink target was cached prior to quarantine, the VFS
> > bypasses that callback entirely when following or reading it. While
> > the target data is still valid, access now needs to be denied.
> >
> > This patch adds an opt-out for symlink caching and enables it for
> > Ceph, handling the encrypted-symlink case for the concurrent series
> > without affecting other filesystems.
> >
>
> That series wasn't sent to me.  Is it
> https://lore.kernel.org/r/cover.1792074165.git.amarkuze@redhat.com/ ?
> It would make more sense to add this patch to that series so that they
> can be reviewed together.  I can potentially apply a prerequisite patch
> by itself, but we'd first need to know that the rest has been reviewed
> and will be accepted too, otherwise it will be unused code.
>
> - Eric
>
Re: [PATCH] fscrypt: allow filesystems to disable symlink target caching
Posted by Xiubo Li 3 days, 13 hours ago
On Thu, Oct 15, 2026 at 05:46:24PM +0000, Alex Markuze wrote:
> fscrypt stores decrypted symlink targets in inode->i_link. The VFS can
> use this field without calling the filesystem's ->get_link() callback.

Agree with Eric that this belongs in the quarantine series, with the
ceph_fscrypt_ops hunk folded into the patch that adds the ->get_link()
check.

I tested it on top of quarantine v3 with a KASAN/lockdep kernel and a
v2-policy encrypted directory.  With v3 alone, the only access to a
cached encrypted symlink that still succeeds during quarantine is
readlinkat() on an O_PATH fd opened before quarantine, with an empty
path.  Path-based readlink/follow/lstat and readlinkat(dirfd, name) are
already refused by the lookup checks.  With this patch the O_PATH case
returns EACCES as well.  Please describe that case in the changelog,
since it is what justifies disabling the cache.

Only the two writers need gating (the kmemdup() in
__fscrypt_encrypt_symlink() and the cmpxchg_release() in
fscrypt_get_symlink()).  If i_link is never populated, the READ_ONCE()s
in fscrypt_get_symlink() and fscrypt_symlink_getattr() already see
NULL, and the VFS reads i_link without looking at the flag anyway.  A
writer-only version behaved identically in the same tests.  Please also
update the fscrypt_symlink_getattr() kernel-doc, which still says that
targets are cached in ->i_link.

The cost is one allocation plus one decryption on every readlink,
follow and lstat of an encrypted symlink: a re-read link went from a
single fscrypt_get_symlink() call to one per access.  That is probably
acceptable, but the changelog should say so.

Thanks,
Xiubo


On Thu, 15 Oct 2026 at 21:45, Alex Markuze <amarkuze@redhat.com> wrote:
>
> I didn't want to conflate fscrypt patches with Ceph features; arguably
> two distinct patchsets.
>
> I will CC you folks when this is acked then.
>
> On Thu, Oct 15, 2026 at 10:38 PM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Thu, Oct 15, 2026 at 09:52:11PM +0300, Alex Markuze wrote:
> > > Hi Eric,
> > >
> > > This supports the concurrent CephFS subvolume quarantine series.
> > > There’s no Fixes: tag since this isn't addressing an existing upstream
> > > regression.
> > >
> > > The quarantine series adds checks to Ceph's ->get_link(). However, if
> > > an encrypted symlink target was cached prior to quarantine, the VFS
> > > bypasses that callback entirely when following or reading it. While
> > > the target data is still valid, access now needs to be denied.
> > >
> > > This patch adds an opt-out for symlink caching and enables it for
> > > Ceph, handling the encrypted-symlink case for the concurrent series
> > > without affecting other filesystems.
> > >
> >
> > That series wasn't sent to me.  Is it
> > https://lore.kernel.org/r/cover.1792074165.git.amarkuze@redhat.com/ ?
> > It would make more sense to add this patch to that series so that they
> > can be reviewed together.  I can potentially apply a prerequisite patch
> > by itself, but we'd first need to know that the rest has been reviewed
> > and will be accepted too, otherwise it will be unused code.
> >
> > - Eric
> >
>
Re: [PATCH] fscrypt: allow filesystems to disable symlink target caching
Posted by Alex Markuze 3 days, 10 hours ago
I''ll resend a combined v4

On Mon, Oct 19, 2026 at 3:56 PM Xiubo Li <xiubo.li@clyso.com> wrote:
>
> On Thu, Oct 15, 2026 at 05:46:24PM +0000, Alex Markuze wrote:
> > fscrypt stores decrypted symlink targets in inode->i_link. The VFS can
> > use this field without calling the filesystem's ->get_link() callback.
>
> Agree with Eric that this belongs in the quarantine series, with the
> ceph_fscrypt_ops hunk folded into the patch that adds the ->get_link()
> check.
>
> I tested it on top of quarantine v3 with a KASAN/lockdep kernel and a
> v2-policy encrypted directory.  With v3 alone, the only access to a
> cached encrypted symlink that still succeeds during quarantine is
> readlinkat() on an O_PATH fd opened before quarantine, with an empty
> path.  Path-based readlink/follow/lstat and readlinkat(dirfd, name) are
> already refused by the lookup checks.  With this patch the O_PATH case
> returns EACCES as well.  Please describe that case in the changelog,
> since it is what justifies disabling the cache.
>
> Only the two writers need gating (the kmemdup() in
> __fscrypt_encrypt_symlink() and the cmpxchg_release() in
> fscrypt_get_symlink()).  If i_link is never populated, the READ_ONCE()s
> in fscrypt_get_symlink() and fscrypt_symlink_getattr() already see
> NULL, and the VFS reads i_link without looking at the flag anyway.  A
> writer-only version behaved identically in the same tests.  Please also
> update the fscrypt_symlink_getattr() kernel-doc, which still says that
> targets are cached in ->i_link.
>
> The cost is one allocation plus one decryption on every readlink,
> follow and lstat of an encrypted symlink: a re-read link went from a
> single fscrypt_get_symlink() call to one per access.  That is probably
> acceptable, but the changelog should say so.
>
> Thanks,
> Xiubo
>
>
> On Thu, 15 Oct 2026 at 21:45, Alex Markuze <amarkuze@redhat.com> wrote:
> >
> > I didn't want to conflate fscrypt patches with Ceph features; arguably
> > two distinct patchsets.
> >
> > I will CC you folks when this is acked then.
> >
> > On Thu, Oct 15, 2026 at 10:38 PM Eric Biggers <ebiggers@kernel.org> wrote:
> > >
> > > On Thu, Oct 15, 2026 at 09:52:11PM +0300, Alex Markuze wrote:
> > > > Hi Eric,
> > > >
> > > > This supports the concurrent CephFS subvolume quarantine series.
> > > > There’s no Fixes: tag since this isn't addressing an existing upstream
> > > > regression.
> > > >
> > > > The quarantine series adds checks to Ceph's ->get_link(). However, if
> > > > an encrypted symlink target was cached prior to quarantine, the VFS
> > > > bypasses that callback entirely when following or reading it. While
> > > > the target data is still valid, access now needs to be denied.
> > > >
> > > > This patch adds an opt-out for symlink caching and enables it for
> > > > Ceph, handling the encrypted-symlink case for the concurrent series
> > > > without affecting other filesystems.
> > > >
> > >
> > > That series wasn't sent to me.  Is it
> > > https://lore.kernel.org/r/cover.1792074165.git.amarkuze@redhat.com/ ?
> > > It would make more sense to add this patch to that series so that they
> > > can be reviewed together.  I can potentially apply a prerequisite patch
> > > by itself, but we'd first need to know that the rest has been reviewed
> > > and will be accepted too, otherwise it will be unused code.
> > >
> > > - Eric
> > >
> >
>