* [PATCH 2/3] netfs: Fix read progress reporting
2026-08-24 12:02 [PATCH 0/3] netfs, cachefiles: Miscellaneous fixes David Howells
2026-08-24 12:02 ` [PATCH 1/3] netfs: Fix uninitialized return value in netfs_unbuffered_write() David Howells
@ 2026-08-24 12:02 ` David Howells
2026-08-24 12:02 ` [PATCH 3/3] cachefiles: Fix potential UAF/KASAN warning David Howells
2 siblings, 0 replies; 4+ messages in thread
From: David Howells @ 2026-08-24 12:02 UTC (permalink / raw)
To: Christian Brauner
Cc: David Howells, Paulo Alcantara, netfs, linux-afs, linux-cifs,
ceph-devel, linux-fsdevel, linux-kernel
For really big read RPC ops that span multiple folios, netfslib allows the
filesystem to give progress notifications to wake up the collector thread
to do a collection of folios that have now been fetched, even if the RPC is
still ongoing, thereby allowing the application to make progress.
This works by taking the current rreq->cleaned_to value (which indicates
which folios have been unlocked) and adding the stashed size of the next
folio to it. cleaned_to, however, is subject to 64-bit tearing on a 32-bit
arch.
Fix this by stashing the next progress notification point as a size_t
(which won't tear) to be added to rreq->start (which won't change), with
the collector thread calculating that from cleaned_to plus the next folio
size.
Further, however, if the folios are small, the collector thread gets
constantly woken up - which has a negative performance impact on the
system.
Fix that too by setting a minimum trigger of 256KiB or the size of the
folio at the front of the queue, whichever is larger.
Also, make sure rreq->cleaned_to is initialised up front, along with
rreq->collected_to and stream->collected_to.
Fixes: e2d46f2ec332 ("netfs: Change the read result collector to only use one work item")
Link: https://sashiko.dev/#/patchset/20260804100224.2748935-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Paulo Alcantara <pc@manguebit.org>
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
---
fs/netfs/buffered_read.c | 4 +++
fs/netfs/internal.h | 1 +
fs/netfs/objects.c | 32 ++++++++++++--------
fs/netfs/read_collect.c | 58 +++++++++++++++++++++++++++---------
fs/netfs/read_single.c | 2 ++
include/linux/netfs.h | 2 +-
include/trace/events/netfs.h | 21 +++++++++++++
7 files changed, 93 insertions(+), 27 deletions(-)
diff --git a/fs/netfs/buffered_read.c b/fs/netfs/buffered_read.c
index 7fdfa4f27e34..3c32ef41a27f 100644
--- a/fs/netfs/buffered_read.c
+++ b/fs/netfs/buffered_read.c
@@ -106,6 +106,9 @@ static ssize_t netfs_prepare_read_iterator(struct netfs_io_subrequest *subreq,
folio_batch_release(&put_batch);
return added;
}
+
+ if (!rreq->progress_at)
+ netfs_read_set_unlock_at(rreq);
rreq->submitted += added;
}
folio_batch_release(&put_batch);
@@ -387,6 +390,7 @@ static int netfs_create_singular_buffer(struct netfs_io_request *rreq, struct fo
if (added < 0)
return added;
rreq->submitted = rreq->start + added;
+ rreq->progress_at = added;
return 0;
}
diff --git a/fs/netfs/internal.h b/fs/netfs/internal.h
index 420ee7b26580..f92281a611de 100644
--- a/fs/netfs/internal.h
+++ b/fs/netfs/internal.h
@@ -109,6 +109,7 @@ static inline void netfs_see_subrequest(struct netfs_io_subrequest *subreq,
/*
* read_collect.c
*/
+void netfs_read_set_unlock_at(struct netfs_io_request *rreq);
bool netfs_read_collection(struct netfs_io_request *rreq);
void netfs_read_collection_worker(struct work_struct *work);
void netfs_cancel_read(struct netfs_io_subrequest *subreq, int error);
diff --git a/fs/netfs/objects.c b/fs/netfs/objects.c
index 01461a74642d..7f6a3e912602 100644
--- a/fs/netfs/objects.c
+++ b/fs/netfs/objects.c
@@ -41,24 +41,32 @@ struct netfs_io_request *netfs_alloc_request(struct address_space *mapping,
memset(rreq, 0, kmem_cache_size(cache));
INIT_WORK(&rreq->cleanup_work, netfs_free_request);
- rreq->gfp = gfp;
- rreq->start = start;
- rreq->len = len;
- rreq->origin = origin;
- rreq->netfs_ops = ctx->ops;
- rreq->mapping = mapping;
- rreq->inode = inode;
- rreq->i_size = i_size_read(inode);
- rreq->debug_id = atomic_inc_return(&debug_ids);
- rreq->wsize = INT_MAX;
+ rreq->gfp = gfp;
+ rreq->start = start;
+ rreq->collected_to = start;
+ rreq->cleaned_to = start;
+ rreq->len = len;
+ rreq->progress_at = 0;
+ rreq->origin = origin;
+ rreq->netfs_ops = ctx->ops;
+ rreq->mapping = mapping;
+ rreq->inode = inode;
+ rreq->i_size = i_size_read(inode);
+ rreq->debug_id = atomic_inc_return(&debug_ids);
+ rreq->wsize = INT_MAX;
rreq->io_streams[0].sreq_max_len = ULONG_MAX;
rreq->io_streams[0].sreq_max_segs = 0;
spin_lock_init(&rreq->lock);
- INIT_LIST_HEAD(&rreq->io_streams[0].subrequests);
- INIT_LIST_HEAD(&rreq->io_streams[1].subrequests);
init_waitqueue_head(&rreq->waitq);
refcount_set(&rreq->ref, 2);
+ for (int s = 0; s < NR_IO_STREAMS; s++) {
+ struct netfs_io_stream *stream = &rreq->io_streams[s];
+
+ INIT_LIST_HEAD(&stream->subrequests);
+ stream->collected_to = rreq->start;
+ }
+
if (origin == NETFS_READAHEAD ||
origin == NETFS_READPAGE ||
origin == NETFS_READ_GAPS ||
diff --git a/fs/netfs/read_collect.c b/fs/netfs/read_collect.c
index 23660a590124..723b479ef606 100644
--- a/fs/netfs/read_collect.c
+++ b/fs/netfs/read_collect.c
@@ -94,6 +94,35 @@ static void netfs_unlock_read_folio(struct netfs_io_request *rreq,
folioq_clear(folioq, slot);
}
+/*
+ * Determine how much to gather before unlocking more folios.
+ */
+void netfs_read_set_unlock_at(struct netfs_io_request *rreq)
+{
+ struct folio_queue *folioq = rreq->buffer.tail;
+ unsigned int slot = rreq->buffer.first_tail_slot;
+ size_t cleaned_to = rreq->cleaned_to - rreq->start;
+ size_t progress_at = cleaned_to;
+ size_t minimum = 256 * 1024;
+
+ while (progress_at < rreq->len) {
+ if (slot >= folioq_nr_slots(folioq)) {
+ folioq = folioq->next;
+ if (!folioq)
+ break;
+ slot = 0;
+ }
+
+ progress_at += folioq_folio_size(folioq, slot);
+ if (progress_at - cleaned_to >= minimum)
+ break;
+ slot++;
+ }
+
+ WRITE_ONCE(rreq->progress_at, progress_at);
+ trace_netfs_read_progress_at(rreq);
+}
+
/*
* Unlock any folios we've finished with.
*/
@@ -112,7 +141,7 @@ static void netfs_read_unlock_folios(struct netfs_io_request *rreq,
if (slot >= folioq_nr_slots(folioq)) {
folioq = rolling_buffer_delete_spent(&rreq->buffer);
if (!folioq) {
- rreq->front_folio_order = 0;
+ WRITE_ONCE(rreq->progress_at, ULONG_MAX);
return;
}
slot = 0;
@@ -120,8 +149,7 @@ static void netfs_read_unlock_folios(struct netfs_io_request *rreq,
for (;;) {
struct folio *folio;
- unsigned long long fpos, fend;
- unsigned int order;
+ unsigned long long fpos = rreq->cleaned_to, fend;
size_t fsize;
if (*notes & COPY_TO_CACHE)
@@ -133,9 +161,7 @@ static void netfs_read_unlock_folios(struct netfs_io_request *rreq,
rreq->debug_id, folio->index))
trace_netfs_folio(folio, netfs_folio_trace_not_locked);
- order = folioq_folio_order(folioq, slot);
- rreq->front_folio_order = order;
- fsize = PAGE_SIZE << order;
+ fsize = folioq_folio_size(folioq, slot);
fpos = folio_pos(folio);
fend = fpos + fsize;
@@ -146,7 +172,7 @@ static void netfs_read_unlock_folios(struct netfs_io_request *rreq,
break;
netfs_unlock_read_folio(rreq, folioq, slot);
- WRITE_ONCE(rreq->cleaned_to, fpos + fsize);
+ WRITE_ONCE(rreq->cleaned_to, fend);
*notes |= MADE_PROGRESS;
clear_bit(NETFS_RREQ_FOLIO_COPY_TO_CACHE, &rreq->flags);
@@ -172,6 +198,8 @@ static void netfs_read_unlock_folios(struct netfs_io_request *rreq,
rreq->buffer.tail = folioq;
done:
rreq->buffer.first_tail_slot = slot;
+
+ netfs_read_set_unlock_at(rreq);
}
/*
@@ -232,7 +260,7 @@ static void netfs_collect_read_results(struct netfs_io_request *rreq)
* subreqs.
*/
if (notes & BUFFERED) {
- size_t fsize = PAGE_SIZE << rreq->front_folio_order;
+ uoff_t unlock_at = rreq->start + rreq->progress_at;
/* Clear the tail of a short read. */
if (!(notes & HIT_PENDING) &&
@@ -257,7 +285,7 @@ static void netfs_collect_read_results(struct netfs_io_request *rreq)
transferred = front->len;
trace_netfs_rreq(rreq, netfs_rreq_trace_set_abandon);
}
- if (front->start + transferred >= rreq->cleaned_to + fsize ||
+ if (front->start + transferred >= unlock_at ||
test_bit(NETFS_SREQ_HIT_EOF, &front->flags))
netfs_read_unlock_folios(rreq, ¬es);
} else {
@@ -477,20 +505,22 @@ void netfs_read_collection_worker(struct work_struct *work)
void netfs_read_subreq_progress(struct netfs_io_subrequest *subreq)
{
struct netfs_io_request *rreq = subreq->rreq;
- struct netfs_io_stream *stream = &rreq->io_streams[0];
- size_t fsize = PAGE_SIZE << rreq->front_folio_order;
-
- trace_netfs_sreq(subreq, netfs_sreq_trace_progress);
+ struct netfs_io_stream *stream = &rreq->io_streams[subreq->stream_nr];
+ size_t progress_at = READ_ONCE(rreq->progress_at);
+ uoff_t update_at = rreq->start + progress_at;
+ uoff_t transferred_to = subreq->start + subreq->transferred;
/* If we are at the head of the queue, wake up the collector,
* getting a ref to it if we were the ones to do so.
*/
- if (subreq->start + subreq->transferred > rreq->cleaned_to + fsize &&
+ if (progress_at != ULONG_MAX &&
+ transferred_to >= update_at &&
(rreq->origin == NETFS_READAHEAD ||
rreq->origin == NETFS_READPAGE ||
rreq->origin == NETFS_READ_FOR_WRITE) &&
list_is_first(&subreq->rreq_link, &stream->subrequests)
) {
+ trace_netfs_sreq(subreq, netfs_sreq_trace_progress);
__set_bit(NETFS_SREQ_MADE_PROGRESS, &subreq->flags);
netfs_wake_collector(rreq);
}
diff --git a/fs/netfs/read_single.c b/fs/netfs/read_single.c
index 8833550d2eb6..de67ac41548d 100644
--- a/fs/netfs/read_single.c
+++ b/fs/netfs/read_single.c
@@ -170,6 +170,8 @@ ssize_t netfs_read_single(struct inode *inode, struct file *file, struct iov_ite
if (IS_ERR(rreq))
return PTR_ERR(rreq);
+ rreq->progress_at = rreq->len;
+
ret = netfs_single_begin_cache_read(rreq, ictx);
if (ret == -ENOMEM || ret == -EINTR || ret == -ERESTARTSYS)
goto cleanup_free;
diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index f837a501008c..a8702bd52d3a 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -246,6 +246,7 @@ struct netfs_io_request {
unsigned long long submitted; /* Amount submitted for I/O so far */
unsigned long long len; /* Length of the request */
size_t transferred; /* Amount to be indicated as transferred */
+ size_t progress_at; /* Report read progress when hit this much read */
long error; /* 0 or error that occurred */
unsigned long long i_size; /* Size of the file */
unsigned long long start; /* Start position */
@@ -262,7 +263,6 @@ struct netfs_io_request {
atomic_t subreq_counter; /* Next subreq->debug_index */
unsigned int nr_group_rel; /* Number of refs to release on ->group */
spinlock_t lock; /* Lock for queuing subreqs */
- unsigned char front_folio_order; /* Order (size) of front folio */
enum netfs_io_origin origin; /* Origin of the request */
bool direct_bv_unpin; /* T if direct_bv[] must be unpinned */
refcount_t ref;
diff --git a/include/trace/events/netfs.h b/include/trace/events/netfs.h
index 082cb03c6131..8ec10c076875 100644
--- a/include/trace/events/netfs.h
+++ b/include/trace/events/netfs.h
@@ -786,6 +786,27 @@ TRACE_EVENT(netfs_folioq,
__print_symbolic(__entry->trace, netfs_folioq_traces))
);
+TRACE_EVENT(netfs_read_progress_at,
+ TP_PROTO(const struct netfs_io_request *rreq),
+
+ TP_ARGS(rreq),
+
+ TP_STRUCT__entry(
+ __field(unsigned int, rreq)
+ __field(size_t, progress_at)
+ __field(size_t, cleaned_to)
+ ),
+
+ TP_fast_assign(
+ __entry->rreq = rreq->debug_id;
+ __entry->cleaned_to = rreq->cleaned_to - rreq->start;
+ __entry->progress_at = rreq->progress_at;
+ ),
+
+ TP_printk("R=%08x cln=%zx prg=%zx",
+ __entry->rreq, __entry->cleaned_to, __entry->progress_at)
+ );
+
#undef EM
#undef E_
#endif /* _TRACE_NETFS_H */
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 3/3] cachefiles: Fix potential UAF/KASAN warning
2026-08-24 12:02 [PATCH 0/3] netfs, cachefiles: Miscellaneous fixes David Howells
2026-08-24 12:02 ` [PATCH 1/3] netfs: Fix uninitialized return value in netfs_unbuffered_write() David Howells
2026-08-24 12:02 ` [PATCH 2/3] netfs: Fix read progress reporting David Howells
@ 2026-08-24 12:02 ` David Howells
2 siblings, 0 replies; 4+ messages in thread
From: David Howells @ 2026-08-24 12:02 UTC (permalink / raw)
To: Christian Brauner
Cc: David Howells, Paulo Alcantara, netfs, linux-afs, linux-cifs,
ceph-devel, linux-fsdevel, linux-kernel
Currently, trace_cachefiles_coherency() is being passed a pointer to a
__be64 lain over the coherency data in struct cachefiles_xattr so that it
can display the first 8 bytes. However, the data is of variable length and
could even be 0 bytes. This could lead to a UAF or KASAN warning.
Fix this by making sure the buffer has room for at least 8 bytes and that
those 8 bytes are pre-cleared.
Further, those bytes are not 8-byte aligned, so fix the tracepoint to
extract the data as four 2-byte words (they are 2-byte aligned) and
reassemble the __be64. The compiler will convert this into a single 8-byte
load where the CPU supports it.
Fixes: 229105e5cfd9 ("cachefiles: Add auxiliary data trace")
Link: https://sashiko.dev/#/patchset/20260810144746.574036-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Paulo Alcantara <pc@manguebit.org>
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
---
fs/cachefiles/xattr.c | 16 ++++++++--------
include/trace/events/cachefiles.h | 19 +++++++++++++++++--
2 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/fs/cachefiles/xattr.c b/fs/cachefiles/xattr.c
index f8ae78b3f7b6..92990cfdcca7 100644
--- a/fs/cachefiles/xattr.c
+++ b/fs/cachefiles/xattr.c
@@ -13,6 +13,7 @@
#include <linux/quotaops.h>
#include <linux/xattr.h>
#include <linux/slab.h>
+#include <linux/unaligned.h>
#include "internal.h"
#define CACHEFILES_COOKIE_TYPE_DATA 1
@@ -50,7 +51,7 @@ int cachefiles_set_object_xattr(struct cachefiles_object *object)
_enter("%x,#%d", object->debug_id, len);
- buf = kmalloc(sizeof(struct cachefiles_xattr) + len, GFP_KERNEL);
+ buf = kmalloc(sizeof(struct cachefiles_xattr) + min(len, sizeof(__be64)), GFP_KERNEL);
if (!buf)
return -ENOMEM;
@@ -60,6 +61,7 @@ int cachefiles_set_object_xattr(struct cachefiles_object *object)
buf->content = object->content_info;
if (test_bit(FSCACHE_COOKIE_LOCAL_WRITE, &object->cookie->flags))
buf->content = CACHEFILES_CONTENT_DIRTY;
+ put_unaligned_be64(0, (__be64 *)buf->data);
if (len > 0)
memcpy(buf->data, fscache_get_aux(object->cookie), len);
@@ -77,8 +79,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),
- buf->content,
+ buf->data, buf->content,
cachefiles_coherency_set_fail);
if (ret != -ENOMEM)
cachefiles_io_error_obj(
@@ -86,8 +87,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),
- buf->content,
+ buf->data, buf->content,
cachefiles_coherency_set_ok);
}
@@ -110,9 +110,10 @@ int cachefiles_check_auxdata(struct cachefiles_object *object, struct file *file
int ret = -ESTALE;
tlen = sizeof(struct cachefiles_xattr) + len;
- buf = kmalloc(tlen, GFP_KERNEL);
+ buf = kmalloc(sizeof(struct cachefiles_xattr) + min(len, sizeof(__be64)), GFP_KERNEL);
if (!buf)
return -ENOMEM;
+ put_unaligned_be64(0, (__be64 *)buf->data);
xlen = cachefiles_inject_read_error();
if (xlen == 0)
@@ -148,8 +149,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);
+ buf->data, buf->content, why);
kfree(buf);
return ret;
}
diff --git a/include/trace/events/cachefiles.h b/include/trace/events/cachefiles.h
index 9259bc71049e..927338f8fe85 100644
--- a/include/trace/events/cachefiles.h
+++ b/include/trace/events/cachefiles.h
@@ -372,7 +372,7 @@ TRACE_EVENT(cachefiles_rename,
TRACE_EVENT(cachefiles_coherency,
TP_PROTO(struct cachefiles_object *obj,
ino_t ino,
- u64 disk_aux,
+ const void *disk_aux,
enum cachefiles_content content,
enum cachefiles_coherency_trace why),
@@ -389,12 +389,27 @@ TRACE_EVENT(cachefiles_coherency,
),
TP_fast_assign(
+ union {
+ __be16 s[4];
+ __be64 ll;
+ } x;
+
__entry->obj = obj->debug_id;
__entry->why = why;
__entry->content = content;
__entry->ino = ino;
__entry->aux = be64_to_cpup((__be64 *)obj->cookie->inline_aux);
- __entry->disk_aux = disk_aux;
+
+ /* cachefiles_xattr::data is not 64-byte aligned. */
+ if (disk_aux) {
+ x.s[0] = ((__be16 *)disk_aux)[0];
+ x.s[1] = ((__be16 *)disk_aux)[1];
+ x.s[2] = ((__be16 *)disk_aux)[2];
+ x.s[3] = ((__be16 *)disk_aux)[3];
+ __entry->disk_aux = be64_to_cpu(x.ll);
+ } else {
+ __entry->disk_aux = 0;
+ }
),
TP_printk("o=%08x %s B=%llx c=%u aux=%llx dsk=%llx",
^ permalink raw reply related [flat|nested] 4+ messages in thread