[PATCH v3 14/19] netfs: Fix folio->private handling in netfs_perform_write()
David Howells
dhowells at redhat.com
Sat Apr 25 05:54:18 PDT 2026
Under some circumstances, netfs_perform_write() doesn't correctly
manipulate folio->private between NULL, NETFS_FOLIO_COPY_TO_CACHE, pointing
to a group and pointing to a netfs_folio struct, leading to potential
multiple attachments of private data with associated folio ref leaks and
also leaks of netfs_folio structs or netfs_group refs.
Fix this by consolidating the place at which a folio is marked uptodate in
one place and having that look at what's attached to folio->private and
decide how to clean it up and then set the new group. Also, the content
shouldn't be flushed if group is NULL, even if a group is specified in the
netfs_group parameter, as that would be the case for a new folio. A
filesystem should always specify netfs_group or never specify netfs_group.
The Sashiko auto-review tool noted that it was theoretically possible that
the fpos >= ctx->zero_point section might leak if it modified a streaming
write folio. This is unlikely, but with a network filesystem, third party
changes can happen. It also pointed out that __netfs_set_group() would
leak if called multiple times on the same folio from the "whole folio
modify section".
Fixes: 8f52de0077ba ("netfs: Reduce number of conditional branches in netfs_perform_write()")
Closes: https://sashiko.dev/#/patchset/20260414082004.3756080-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells at redhat.com>
cc: Paulo Alcantara <pc at manguebit.org>
cc: Matthew Wilcox <willy at infradead.org>
cc: netfs at lists.linux.dev
cc: linux-fsdevel at vger.kernel.org
---
fs/netfs/buffered_write.c | 77 +++++++++++++++++++++---------------
include/trace/events/netfs.h | 2 +
2 files changed, 48 insertions(+), 31 deletions(-)
diff --git a/fs/netfs/buffered_write.c b/fs/netfs/buffered_write.c
index c7b49b38a710..0439a4c2e003 100644
--- a/fs/netfs/buffered_write.c
+++ b/fs/netfs/buffered_write.c
@@ -12,12 +12,6 @@
#include <linux/slab.h>
#include "internal.h"
-static void __netfs_set_group(struct folio *folio, struct netfs_group *netfs_group)
-{
- if (netfs_group)
- folio_attach_private(folio, netfs_get_group(netfs_group));
-}
-
static void netfs_set_group(struct folio *folio, struct netfs_group *netfs_group)
{
void *priv = folio_get_private(folio);
@@ -157,6 +151,7 @@ ssize_t netfs_perform_write(struct kiocb *iocb, struct iov_iter *iter,
size_t offset; /* Offset into pagecache folio */
size_t part; /* Bytes to write to folio */
size_t copied; /* Bytes copied from user */
+ void *priv;
offset = pos & (max_chunk - 1);
part = min(max_chunk - offset, iov_iter_count(iter));
@@ -212,8 +207,9 @@ ssize_t netfs_perform_write(struct kiocb *iocb, struct iov_iter *iter,
finfo = netfs_folio_info(folio);
group = netfs_folio_group(folio);
- if (unlikely(group != netfs_group) &&
- group != NETFS_FOLIO_COPY_TO_CACHE)
+ if (unlikely(group) &&
+ group != NETFS_FOLIO_COPY_TO_CACHE &&
+ group != netfs_group)
goto flush_content;
if (folio_test_uptodate(folio)) {
@@ -237,24 +233,22 @@ ssize_t netfs_perform_write(struct kiocb *iocb, struct iov_iter *iter,
if (unlikely(copied == 0))
goto copy_failed;
folio_zero_segment(folio, offset + copied, flen);
- __netfs_set_group(folio, netfs_group);
- folio_mark_uptodate(folio);
- trace = netfs_modify_and_clear;
- goto copied;
+ if (finfo)
+ trace = netfs_modify_and_clear_rm_finfo;
+ else
+ trace = netfs_modify_and_clear;
+ goto mark_uptodate;
}
/* See if we can write a whole folio in one go. */
if (!maybe_trouble && offset == 0 && part >= flen) {
copied = copy_folio_from_iter_atomic(folio, offset, part, iter);
if (likely(copied == part)) {
- if (finfo) {
+ if (finfo)
trace = netfs_whole_folio_modify_filled;
- goto folio_now_filled;
- }
- __netfs_set_group(folio, netfs_group);
- folio_mark_uptodate(folio);
- trace = netfs_whole_folio_modify;
- goto copied;
+ else
+ trace = netfs_whole_folio_modify;
+ goto mark_uptodate;
}
if (copied == 0)
goto copy_failed;
@@ -270,8 +264,10 @@ ssize_t netfs_perform_write(struct kiocb *iocb, struct iov_iter *iter,
* accept the partial write.
*/
finfo->dirty_len += finfo->dirty_offset;
- if (finfo->dirty_len == flen)
- goto folio_now_filled;
+ if (finfo->dirty_len == flen) {
+ trace = netfs_whole_folio_modify_filled_efault;
+ goto mark_uptodate;
+ }
if (copied > finfo->dirty_len)
finfo->dirty_len = copied;
finfo->dirty_offset = 0;
@@ -303,6 +299,7 @@ ssize_t netfs_perform_write(struct kiocb *iocb, struct iov_iter *iter,
goto copied;
}
+ /* Do a streaming write on a folio that has nothing in it yet. */
if (!finfo) {
ret = -EIO;
if (WARN_ON(folio_get_private(folio)))
@@ -311,10 +308,8 @@ ssize_t netfs_perform_write(struct kiocb *iocb, struct iov_iter *iter,
if (unlikely(copied == 0))
goto copy_failed;
if (offset == 0 && copied == flen) {
- __netfs_set_group(folio, netfs_group);
- folio_mark_uptodate(folio);
trace = netfs_streaming_filled_page;
- goto copied;
+ goto mark_uptodate;
}
finfo = kzalloc_obj(*finfo);
@@ -343,7 +338,7 @@ ssize_t netfs_perform_write(struct kiocb *iocb, struct iov_iter *iter,
finfo->dirty_len += copied;
if (finfo->dirty_offset == 0 && finfo->dirty_len == flen) {
trace = netfs_streaming_cont_filled_page;
- goto folio_now_filled;
+ goto mark_uptodate;
}
trace = netfs_streaming_write_cont;
goto copied;
@@ -359,13 +354,33 @@ ssize_t netfs_perform_write(struct kiocb *iocb, struct iov_iter *iter,
goto out;
continue;
- folio_now_filled:
- if (finfo->netfs_group)
- folio_change_private(folio, finfo->netfs_group);
- else
- folio_detach_private(folio);
+ /* Mark a folio as being up to data when we've filled it
+ * completely. If the folio has a group attached, then it must
+ * be the same group, otherwise we should have flushed it out
+ * above. We have to get rid of the netfs_folio struct if
+ * there was one.
+ */
+ mark_uptodate:
+ priv = folio_get_private(folio);
+ if (likely(priv == netfs_group)) {
+ /* Already set correctly; no change required. */
+ } else if (priv == NETFS_FOLIO_COPY_TO_CACHE) {
+ if (!netfs_group)
+ folio_detach_private(folio);
+ else
+ folio_change_private(folio, netfs_get_group(netfs_group));
+ } else if (!priv) {
+ folio_attach_private(folio, netfs_get_group(netfs_group));
+ } else {
+ WARN_ON_ONCE(!finfo);
+ if (netfs_group)
+ /* finfo->netfs_group has a ref */
+ folio_change_private(folio, netfs_group);
+ else
+ folio_detach_private(folio);
+ kfree(finfo);
+ }
folio_mark_uptodate(folio);
- kfree(finfo);
copied:
trace_netfs_folio(folio, trace);
flush_dcache_folio(folio);
diff --git a/include/trace/events/netfs.h b/include/trace/events/netfs.h
index 67f6d56c94ce..1f5e3a5af08a 100644
--- a/include/trace/events/netfs.h
+++ b/include/trace/events/netfs.h
@@ -179,7 +179,9 @@
EM(netfs_whole_folio_modify, "mod-whole-f") \
EM(netfs_whole_folio_modify_efault, "mod-whole-f!") \
EM(netfs_whole_folio_modify_filled, "mod-whole-f+") \
+ EM(netfs_whole_folio_modify_filled_efault, "mod-whole-f+!") \
EM(netfs_modify_and_clear, "mod-n-clear") \
+ EM(netfs_modify_and_clear_rm_finfo, "mod-n-clear+") \
EM(netfs_streaming_write, "mod-streamw") \
EM(netfs_streaming_write_cont, "mod-streamw+") \
EM(netfs_flush_content, "flush") \
More information about the linux-afs
mailing list