[PATCH] fs: do not cache a symlink length that disagrees with the string

[Narek Jilavyan <njilav@gmail.com>]

inode_set_cached_link() stores a caller-supplied length in i_linklen and
sets IOP_CACHED_LINK. vfs_readlink() then uses that length directly:

	if (inode->i_opflags & IOP_CACHED_LINK)
		return readlink_copy(buffer, buflen, inode->i_link,
				     inode->i_linklen);

and readlink_copy() clamps only against the user buffer, not against
the string, so a length larger than the symlink body becomes a
copy_to_user() of adjacent kernel memory - reachable by any process
calling readlink() on such a symlink.

The only thing standing behind the invariant is

	VFS_WARN_ON_INODE(strlen(link) != linklen, inode);

which expands to BUILD_BUG_ON_INVALID() unless CONFIG_DEBUG_VFS is set.
On a production kernel it type-checks the expression and evaluates
nothing, so the value is stored unvalidated.

All four in-tree callers are correct today, and notably the two whose
length comes from on-disk metadata (fs/ext4/inode.c, fs/erofs/inode.c)
both re-derive it and reject the inode rather than relying on this
helper.  The API should not require that of the next caller.

Validate unconditionally and fail safe: if the length disagrees, warn
and leave IOP_CACHED_LINK clear.  i_linklen has exactly one reader in
the tree and it is gated on that flag, and vfs_readlink() falls back to
i_link with a strlen() of its own, so the inode degrades to the
behaviour that predates the cached length instead of disclosing memory.

The check runs once per symlink inode setup, not once per readlink(),
which is what the cache was for.

Fixes: ea3821990719 ("vfs: support caching symlink lengths in inodes")
Signed-off-by: Narek Jilavyan <njilav@gmail.com>
---
 include/linux/fs.h | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 50ce731a2..e1d8f2614 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -946,9 +946,19 @@ static inline void inode_state_replace(struct inode *inode,
 
 static inline void inode_set_cached_link(struct inode *inode, char *link, int linklen)
 {
-	VFS_WARN_ON_INODE(strlen(link) != linklen, inode);
 	VFS_WARN_ON_INODE(inode->i_opflags & IOP_CACHED_LINK, inode);
 	inode->i_link = link;
+
+	/*
+	 * i_linklen is used as a copy_to_user() length by vfs_readlink(), so it
+	 * must not be taken on trust. If it disagrees with the string, leave
+	 * IOP_CACHED_LINK clear: vfs_readlink() then falls back to i_link and
+	 * recomputes the length with strlen(), which is what it did before the
+	 * cached length was introduced.
+	 */
+	if (WARN_ON_ONCE(strlen(link) != linklen))
+		return;
+
 	inode->i_linklen = linklen;
 	inode->i_opflags |= IOP_CACHED_LINK;
 }
-- 
2.43.0