fs/ceph/crypto.c | 1 + fs/crypto/hooks.c | 37 ++++++++++++++++++++++--------------- include/linux/fscrypt.h | 7 +++++++ 3 files changed, 30 insertions(+), 15 deletions(-)
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
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
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 >
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
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 >
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 > > >
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 > > > > > >
© 2016 - 2026 Red Hat, Inc.