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 <[email protected]> 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 <[email protected]> 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/[email protected]/ ? > > 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 > > >
