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

[Alex Markuze]

I''ll resend a combined v4

On Mon, Oct 19, 2026 at 3:56 PM Xiubo Li <xiubo.li@xxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx/ ?
> > > 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
> > >
> >
>