[PATCH] cachefiles: fix out-of-bounds read in coherency trace

[PATCH] cachefiles: fix out-of-bounds read in coherency trace

[Yichong Chen]

cachefiles traces the netfs coherency data stored in the object xattr by
reading the xattr data field as a 64-bit big-endian value.  The coherency
data length is controlled by the netfs cookie and may be shorter than 8
bytes.

For example, 9p uses a 4-byte qid version as the cookie aux data.  When the
cachefiles_coherency tracepoint is enabled, cachefiles_set_object_xattr()
then reads past the end of the allocated xattr buffer, which KASAN reports
as a slab-out-of-bounds read.

Copy at most the available aux bytes into a zero-padded 64-bit value before
tracing it.  Also avoid reading xattr buffer fields on the getxattr failure
path, where the buffer contents are not valid.

Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
---
 fs/cachefiles/xattr.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/fs/cachefiles/xattr.c b/fs/cachefiles/xattr.c
index f8ae78b3f7b6..2e21abedc83a 100644
--- a/fs/cachefiles/xattr.c
+++ b/fs/cachefiles/xattr.c
@@ -33,6 +33,15 @@ struct cachefiles_vol_xattr {
 	__u8	data[];		/* netfs volume coherency data */
 } __packed;
 
+static u64 cachefiles_xattr_get_aux(const struct cachefiles_xattr *buf,
+				    unsigned int len)
+{
+	__be64 aux = 0;
+
+	memcpy(&aux, buf->data, min_t(unsigned int, len, sizeof(aux)));
+	return be64_to_cpu(aux);
+}
+
 /*
  * set the state xattr on a cache file
  */
@@ -77,7 +86,7 @@ int cachefiles_set_object_xattr(struct cachefiles_object *object)
 		trace_cachefiles_vfs_error(object, file_inode(file), ret,
 					   cachefiles_trace_setxattr_error);
 		trace_cachefiles_coherency(object, file_inode(file)->i_ino,
-					   be64_to_cpup((__be64 *)buf->data),
+					   cachefiles_xattr_get_aux(buf, len),
 					   buf->content,
 					   cachefiles_coherency_set_fail);
 		if (ret != -ENOMEM)
@@ -86,7 +95,7 @@ int cachefiles_set_object_xattr(struct cachefiles_object *object)
 				"Failed to set xattr with error %d", ret);
 	} else {
 		trace_cachefiles_coherency(object, file_inode(file)->i_ino,
-					   be64_to_cpup((__be64 *)buf->data),
+					   cachefiles_xattr_get_aux(buf, len),
 					   buf->content,
 					   cachefiles_coherency_set_ok);
 	}
@@ -106,6 +115,8 @@ int cachefiles_check_auxdata(struct cachefiles_object *object, struct file *file
 	unsigned int len = object->cookie->aux_len, tlen;
 	const void *p = fscache_get_aux(object->cookie);
 	enum cachefiles_coherency_trace why;
+	enum cachefiles_content content = CACHEFILES_CONTENT_NO_DATA;
+	u64 disk_aux = 0;
 	ssize_t xlen;
 	int ret = -ESTALE;
 
@@ -131,6 +142,9 @@ int cachefiles_check_auxdata(struct cachefiles_object *object, struct file *file
 		goto out;
 	}
 
+	content = buf->content;
+	disk_aux = cachefiles_xattr_get_aux(buf, len);
+
 	if (buf->type != CACHEFILES_COOKIE_TYPE_DATA) {
 		why = cachefiles_coherency_check_type;
 	} else if (memcmp(buf->data, p, len) != 0) {
@@ -148,8 +162,7 @@ int cachefiles_check_auxdata(struct cachefiles_object *object, struct file *file
 
 out:
 	trace_cachefiles_coherency(object, file_inode(file)->i_ino,
-				   be64_to_cpup((__be64 *)buf->data),
-				   buf->content, why);
+				   disk_aux, content, why);
 	kfree(buf);
 	return ret;
 }
-- 
2.51.0

Re: [PATCH] cachefiles: fix out-of-bounds read in coherency trace

[David Howells]

Thanks, but I have a patch for this.  See patch 9 here:

https://lore.kernel.org/linux-fsdevel/20260825132045.1000787-1-dhowells@redhat.com/

I don't want to call memcpy() like this just for the purposes of a tracepoint
that is almost certainly not actually be active.

You're also moving buf->content into a variable rather than using it directly
- is there a reason for that?

David

Re: [PATCH] cachefiles: fix out-of-bounds read in coherency trace

[Yichong Chen]

On Thu, Aug 27, 2026 at 08:31:18AM +0100, David Howells wrote:
> Thanks, but I have a patch for this.  See patch 9 here:
>
> https://lore.kernel.org/linux-fsdevel/20260825132045.1000787-1-dhowells@redhat.com/
>
> I don't want to call memcpy() like this just for the purposes of a tracepoint
> that is almost certainly not actually be active.
>
> You're also moving buf->content into a variable rather than using it directly
> - is there a reason for that?

Thanks for pointing me to your patch.

The local content variable was intended to avoid reading fields from the
xattr buffer on the getxattr failure path, where the buffer contents may
not be valid.

Your patch already handles the short auxiliary data case by making room for
the traced bytes and pre-clearing them, so please ignore my patch.  I'll
drop it on my side.

Thanks,
Yichong