netfs, cachefiles: Miscellaneous fixes
Subsystems (8)
David Howells
maintainerv2 cover netfs, cachefiles: Miscellaneous fixes
Hi Christian,
Here are some miscellaneous fixes for netfslib and one for cachefiles, if you could pick them up?
The first six are all in unbuffered/DIO write:
(1) Fix an uninitialised return value from netfs_unbuffered_write().
(2) Fix the normal error return, preferring partial transfer size over request error over immediate error (e.g. EINTR).
(3) Fix the async error return, preferring partial transfer size over error.
(4) Fix the update of i_size on partial transfer ending in an error.
(5) Fix a subrequest leak in an error path.
(6) Fix the code to handle subrequest allocation failure.
Then there's:
(7) Fix synchronisation issues with using a progressive rolling buffer for readahead by fetching everything into it upfront so that the issues don't arise. Dropping the refs so acquired is deferred until after I/O is begun.
(8) Fix read progress reporting to avoid 64-bit tearing on a 32-bit machine. This has been modified from the previous submission as part of a different series to take account of a sashiko reported issue[1].
(9) Fix a potential UAF/KASAN warning reported by sashiko[1] in cachefiles in which the coherency data buffer is cast to a __be64* and dereferenced in a tracepoint - even though it might not be at least that large (or aligned).
The patches can also be found here:
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=netfs-fixes
Thanks,
David
Changes
=======
ver #2)
- Imported a patch to add subreq alloc failure handling in unbuffered/DIO write.
- Fixed a number of issues reported by Sashiko[2]:
- Added four patches to fix bugs in unbuffered/DIO write.
- Added a patch to fix readahead rolling buffer issues.
- Fixed the cachefiles xattr stuff to use max() not min() when allocating.
- Fixed the cachefiles trace comment with regard to alignment.
- Fixed netfs_read_set_unlock_at() to use folioq_count(), not
folio_nr_slots().
[1] https://sashiko.dev/#/patchset/20260810144746.574036-1-dhowells%40redhat.com
[2] https://sashiko.dev/#/patchset/20260824120224.504575-1-dhowells%40redhat.com
David Howells (7):
netfs: Fix unbuffered/DIO write partial transfer error return
netfs: Fix error vs transferred passed to ->ki_complete()
netfs: Fix i_size update for partial transfer
netfs: Fix subreq ref leak
netfs: Fix readahead synchronisation issues by loading all folios
upfront
netfs: Fix read progress reporting
cachefiles: Fix potential UAF/KASAN warning
Edward Adam Davis (1):
netfs: break unbuffered write when netfs_alloc_subrequest() fails
Karl Mehltretter (1):
netfs: Fix uninitialized return value in netfs_unbuffered_write()
fs/cachefiles/xattr.c | 16 ++--- fs/netfs/buffered_read.c | 103 ++++++++++++++++++------------ fs/netfs/direct_write.c | 30 +++++---- fs/netfs/internal.h | 2 + fs/netfs/misc.c | 19 ++++++ fs/netfs/objects.c | 32 ++++++---- fs/netfs/read_collect.c | 65 +++++++++++++++---- fs/netfs/read_retry.c | 7 ++ fs/netfs/read_single.c | 2 + fs/netfs/rolling_buffer.c | 81 ++++++++++++++--------- fs/netfs/write_issue.c | 2 + include/linux/netfs.h | 3 +- include/linux/rolling_buffer.h | 6 +- include/trace/events/cachefiles.h | 19 +++++- include/trace/events/netfs.h | 24 +++++++ 15 files changed, 291 insertions(+), 120 deletions(-)
David Howells
maintainerPATCH v2 1/9 netfs: Fix uninitialized return value in netfs_unbuffered_write()
From: Karl Mehltretter <[email protected]>
If preparation of the first subrequest fails, netfs_unbuffered_write() exits its loop before ret is initialized. The empty-iterator check can do the same.
For synchronous writes, netfs_unbuffered_write_iter_locked() may then return an unrelated error instead of wreq->error. This is reachable through CIFS if cifs_prepare_write() fails to reopen the file or obtain credits.
Initialize ret to 0 so the caller returns wreq->error if no data was written, or the number of bytes already written otherwise.
Found with Clang's -Wconditional-uninitialized.
--- fs/netfs/direct_write.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/netfs/direct_write.c +1 -1
@@ -95,7 +95,7 @@ static int netfs_unbuffered_write(struct netfs_io_request *wreq) {struct netfs_io_subrequest *subreq =NULL ;struct netfs_io_stream *stream = &wreq->io_streams[0 ]; -int ret; +int ret =0 ;_enter ("%llx" , wreq->len);
David Howells
maintainerPATCH v2 2/9 netfs: Fix unbuffered/DIO write partial transfer error return
In reply to David HowellsFix unbuffered/DIO write to return the amount of data transferred in preference to an error if a partial transfer has been achieved, and to prefer an error stashed in the request over the one returned by netfs_unbuffered_write() (likely -EINTR or -ERESTARTSYS).
--- fs/netfs/direct_write.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
fs/netfs/direct_write.c +5 -7
@@ -139,13 +139,11 @@ static int netfs_unbuffered_write(struct netfs_io_request *wreq)if (test_bit (NETFS_SREQ_NEED_RETRY , &subreq->flags)) { retry = true; }else if (test_bit (NETFS_SREQ_FAILED , &subreq->flags)) { - ret = subreq->error; - wreq->error = ret; + wreq->error = subreq->error;netfs_see_subrequest (subreq, netfs_sreq_trace_see_failed); subreq =NULL ;break ; } - ret =0 ;if (!retry) {netfs_unbuffered_write_collect (wreq, stream, subreq);
@@ -288,11 +286,11 @@ ssize_t netfs_unbuffered_write_iter_locked(struct kiocb *iocb, struct iov_iter * ret = -EIOCBQUEUED ; }else { ret =netfs_unbuffered_write (wreq); -if (ret <0 ) { -_debug ("begin = %zd" , ret); - }else { +if (wreq->transferred) { iocb->ki_pos += wreq->transferred; - ret = wreq->transferred ?: wreq->error; + ret = wreq->transferred; + }else if (wreq->error) { + ret = wreq->error; }netfs_put_request (wreq, netfs_rreq_trace_put_complete);
David Howells
maintainerPATCH v2 3/9 netfs: Fix error vs transferred passed to ->ki_complete()
In reply to David HowellsFix netfs_unbuffered_write_done() to pass the amount written to ->ki_complete() rather than the error in the event of a partially complete transfer.
--- fs/netfs/direct_write.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/netfs/direct_write.c +1 -1
@@ -51,7 +51,7 @@ static void netfs_unbuffered_write_done(struct netfs_io_request *wreq) wreq->iocb->ki_pos += written;if (wreq->iocb->ki_complete) {trace_netfs_rreq (wreq, netfs_rreq_trace_ki_complete); - wreq->iocb->ki_complete (wreq->iocb, wreq->error ?: written); + wreq->iocb->ki_complete (wreq->iocb, written ?: wreq->error); } wreq->iocb =VFS_PTR_POISON ; }
David Howells
maintainerPATCH v2 4/9 netfs: Fix i_size update for partial transfer
In reply to David HowellsFix netfs_unbuffered_write_done() to pass the amount written to netfs_update_i_size() in the event of a partial transfer that ends in an error.
That said, it might be better for the filesystem to mark the inode data as invalid and recheck it in case something like a network error occurred that prevented the reply from the server from being received.
--- fs/netfs/direct_write.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/netfs/direct_write.c +1 -1
@@ -21,7 +21,7 @@ static void netfs_unbuffered_write_done(struct netfs_io_request *wreq)/* Okay, declare that all I/O is complete. */ trace_netfs_rreq (wreq, netfs_rreq_trace_write_done); -if (!wreq->error) +if (wreq->transferred)netfs_update_i_size (ictx, &ictx->inode, wreq->start, wreq->transferred); if (wreq->origin == NETFS_DIO_WRITE &&
David Howells
maintainerPATCH v2 5/9 netfs: Fix subreq ref leak
In reply to David HowellsFix a subrequest ref leak in netfs_unbuffered_write() in the event that subreq->io_iter ends up zero length during preparation.
--- fs/netfs/direct_write.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
fs/netfs/direct_write.c +7 -1
@@ -121,8 +121,14 @@ static int netfs_unbuffered_write(struct netfs_io_request *wreq) }iov_iter_truncate (&subreq->io_iter, wreq->len - wreq->transferred); -if (!iov_iter_count (&subreq->io_iter)) +if (!iov_iter_count (&subreq->io_iter)) { +pr_warn ("netfs: Unexpected zero-length iterator R=%08x\n" , + wreq->debug_id); +__set_bit (NETFS_SREQ_FAILED , &subreq->flags); +netfs_write_subrequest_terminated (subreq, -EIO ); + wreq->error = -EIO ;break ; + } subreq->len = netfs_limit_iter(&subreq->io_iter, 0, stream->sreq_max_len,
David Howells
maintainerPATCH v2 6/9 netfs: break unbuffered write when netfs_alloc_subrequest() fails
In reply to David HowellsFrom: Edward Adam Davis <[email protected]>
syzbot reported a null-ptr-deref below [1] following a fault injection in netfs_alloc_subrequest(). [0]
When netfs_alloc_subrequest() fails, subreq is NULL. Later, netfs_prepare_write() tries to initialize members of subreq(e.g., source), the issue in [1] is triggered.
Let's handle the error of netfs_prepare_write() properly.
[0]FAULT_INJECTION: forcing a failure.
name failslab, interval 1, probability 0, space 0, times 0
Call Trace: netfs_alloc_subrequest+0x116/0x3f0 netfs_prepare_write+0x76/0x7b0 netfs_unbuffered_write+0x75c/0x2020 netfs_unbuffered_write_iter_locked+0x7d6/0xa80 netfs_unbuffered_write_iter+0x442/0x720 v9fs_file_write_iter+0xbf/0x100 vfs_write+0x6ac/0x1050
[1]
KASAN: null-ptr-deref in range [0x00000000000000a8-0x00000000000000af] RIP: 0010:netfs_prepare_write+0xbc/0x7b0 fs/netfs/write_issue.c:173 Call Trace: netfs_unbuffered_write+0x75c/0x2020 fs/netfs/direct_write.c:111 netfs_unbuffered_write_iter_locked+0x7d6/0xa80 fs/netfs/direct_write.c:290 netfs_unbuffered_write_iter+0x442/0x720 fs/netfs/direct_write.c:382 v9fs_file_write_iter+0xbf/0x100 fs/9p/vfs_file.c:409 new_sync_write fs/read_write.c:595 [inline]
[dhowells: Altered to put -ENOMEM into ret, not wreq->error]
--- fs/netfs/direct_write.c | 4 ++++ fs/netfs/write_issue.c | 2 ++ 2 files changed, 6 insertions(+)
fs/netfs/direct_write.c +4 -0
@@ -110,6 +110,10 @@ static int netfs_unbuffered_write(struct netfs_io_request *wreq)if (!subreq) {netfs_prepare_write (wreq, stream, wreq->start + wreq->transferred); subreq = stream->construct; +if (!subreq) { + ret = -ENOMEM ; +break ; + } stream->construct =NULL ; }
fs/netfs/write_issue.c +2 -0
@@ -170,6 +170,8 @@ void netfs_prepare_write(struct netfs_io_request *wreq,rolling_buffer_make_space (&wreq->buffer, wreq->gfp); subreq =netfs_alloc_subrequest (wreq); +if (!subreq) +return ; subreq->source = stream->source; subreq->start = start; subreq->stream_nr = stream->stream_nr;
David Howells
maintainerPATCH v2 7/9 netfs: Fix readahead synchronisation issues by loading all folios upfront
In reply to David HowellsThere are some synchronisation issues that derive from the app thread adding more folios to the rolling buffer whilst the collector thread is looking at them or trying to clear them, such as determining the setting of front_folio_order when the next folio hasn't been added yet,
The reason for the rolling buffer approach is that loading the buffer upfront and then dropping all the refs just acquired is quite a slow operation, and loading progressively allows some of the cost to be deferred until after at least some of the I/O is started.
Instead, a better way is to load all the folios into the rolling buffer upfront - and then drop the refs later, once the I/O is in progress. (Even better would be for the refs not to be there at all.)
Fix this by changing the rolling buffer loader to load all the folios selected by the VM for readahead upfront into the folio queue. The folio queue is allocated a batch worth at a time as we don't know how many folios are involved (the readahead_control struct, alas, has a page count, not a folio count).
The folio refs acquired from readahead are then dropped in bulk once the first subrequest is dispatched as it's quite a slow operation. The collector waits for NETFS_RREQ_NEED_PUT_RA_REFS to be cleared so that it doesn't unlock folios before the xarray has been scanned for them.
This simplifies the buffer handling later and isn't noticeably slower as the xarray doesn't need to be modified and the folios are all already pre-locked.
--- fs/netfs/buffered_read.c | 101 ++++++++++++++++++++------------- fs/netfs/internal.h | 1 + fs/netfs/misc.c | 19 +++++++ fs/netfs/read_collect.c | 7 +++ fs/netfs/read_retry.c | 7 +++ fs/netfs/rolling_buffer.c | 81 ++++++++++++++++---------- include/linux/netfs.h | 1 + include/linux/rolling_buffer.h | 6 +- include/trace/events/netfs.h | 3 + 9 files changed, 154 insertions(+), 72 deletions(-)
fs/netfs/buffered_read.c +61 -40
@@ -54,6 +54,42 @@ static void netfs_rreq_expand(struct netfs_io_request *rreq, } } +/* +* Drop the folio refs acquired from the readahead API. +*/ +static void netfs_bulk_drop_ra_refs (struct netfs_io_request *rreq) +{ +struct folio_batch fbatch; +struct folio *folio; +pgoff_t nr_pages =DIV_ROUND_UP (rreq->len,PAGE_SIZE ); +pgoff_t first = rreq->start /PAGE_SIZE ; +XA_STATE (xas, &rreq->mapping->i_pages, first); + +folio_batch_init (&fbatch); + +rcu_read_lock (); + +xas_for_each (&xas, folio, first + nr_pages -1 ) { +if (xas_retry (&xas, folio)) +continue ; + +if (!folio_batch_add (&fbatch, folio)) +folio_batch_release (&fbatch); + } + +rcu_read_unlock (); +folio_batch_release (&fbatch); +trace_netfs_rreq (rreq, netfs_rreq_trace_ra_put_ref); +clear_bit_unlock (NETFS_RREQ_NEED_PUT_RA_REFS , &rreq->flags); +wake_up (&rreq->waitq); +} + +static void netfs_maybe_bulk_drop_ra_refs (struct netfs_io_request *rreq) +{ +if (test_bit (NETFS_RREQ_NEED_PUT_RA_REFS , &rreq->flags)) +netfs_bulk_drop_ra_refs (rreq); +} +/* * Begin an operation, and fetch the stored zero point value from the cookie if * available.
@@ -74,12 +110,8 @@ static int netfs_begin_cache_read(struct netfs_io_request *rreq, struct netfs_in* * Returns the limited size if successful and -ENOMEM if insufficient memory * available. -* -* [!] NOTE: This must be run in the same thread as ->issue_read() was called -* in as we access the readahead_control struct. */ -static ssize_t netfs_prepare_read_iterator (struct netfs_io_subrequest *subreq, -struct readahead_control *ractl) +static ssize_t netfs_prepare_read_iterator (struct netfs_io_subrequest *subreq) {struct netfs_io_request *rreq = subreq->rreq;size_t rsize = subreq->len;
@@ -87,30 +119,6 @@ static ssize_t netfs_prepare_read_iterator(struct netfs_io_subrequest *subreq,if (subreq->source ==NETFS_DOWNLOAD_FROM_SERVER ) rsize =umin (rsize, rreq->io_streams[0 ].sreq_max_len); -if (ractl) { -/* If we don't have sufficient folios in the rolling buffer, -* extract a folioq's worth from the readahead region at a time -* into the buffer. Note that this acquires a ref on each page -* that we will need to release later - but we don't want to do -* that until after we've started the I/O. -*/ -struct folio_batch put_batch; - -folio_batch_init (&put_batch); -while (rreq->submitted < subreq->start + rsize) { -ssize_t added; - - added =rolling_buffer_load_from_ra (&rreq->buffer, ractl, - &put_batch); -if (added <0 ) { -folio_batch_release (&put_batch); -return added; - } - rreq->submitted += added; - } -folio_batch_release (&put_batch); - } - subreq->len = rsize;if (unlikely (rreq->io_streams[0 ].sreq_max_segs)) { size_t limit = netfs_limit_iter(&rreq->buffer.iter, 0, rsize,
@@ -208,8 +216,7 @@ static void netfs_issue_read(struct netfs_io_request *rreq,* slicing up the region to be read according to available cache blocks and * network rsize. */ -static void netfs_read_to_pagecache (struct netfs_io_request *rreq, -struct readahead_control *ractl) +static void netfs_read_to_pagecache (struct netfs_io_request *rreq) {unsigned long long start = rreq->start;ssize_t size = rreq->len;
@@ -288,7 +295,7 @@ static void netfs_read_to_pagecache(struct netfs_io_request *rreq,break ; issue: - slice =netfs_prepare_read_iterator (subreq, ractl); + slice =netfs_prepare_read_iterator (subreq);if (slice <0 ) { ret = slice;netfs_cancel_read (subreq, ret);
@@ -302,6 +309,7 @@ static void netfs_read_to_pagecache(struct netfs_io_request *rreq, }netfs_issue_read (rreq, subreq); +netfs_maybe_bulk_drop_ra_refs (rreq);if (test_bit (NETFS_RREQ_PAUSE , &rreq->flags))netfs_wait_for_paused_read (rreq);
@@ -339,7 +347,8 @@ void netfs_readahead(struct readahead_control *ractl) {struct netfs_io_request *rreq;struct netfs_inode *ictx =netfs_inode (ractl->mapping->host); -unsigned long long start =readahead_pos (ractl); +ssize_t added; +uoff_t start =readahead_pos (ractl);size_t size =readahead_length (ractl);int ret;
@@ -360,11 +369,23 @@ void netfs_readahead(struct readahead_control *ractl)netfs_rreq_expand (rreq, ractl); - rreq->submitted = rreq->start; -if (rolling_buffer_init (&rreq->buffer, rreq->debug_id,ITER_DEST , rreq->gfp) <0 ) +/* Load the folios to be read into a bvecq chain. Note that this +* acquires a ref on each folio that we will need to release later - +* but we don't want to do that until after we've started the I/O. +*/ + added =rolling_buffer_bulk_load_from_ra (&rreq->buffer, ractl, + rreq->debug_id, rreq->gfp); +if (added <0 ) { + ret = added; goto cleanup_free; -netfs_read_to_pagecache (rreq, ractl); + } +__set_bit (NETFS_RREQ_NEED_PUT_RA_REFS , &rreq->flags); + + rreq->submitted = rreq->start + added; + rreq->cleaned_to = rreq->start; +netfs_read_to_pagecache (rreq); +netfs_maybe_bulk_drop_ra_refs (rreq);return netfs_put_request (rreq, netfs_rreq_trace_put_return); cleanup_free:
@@ -457,7 +478,7 @@ static int netfs_read_gaps(struct file *file, struct folio *folio)iov_iter_bvec (&rreq->buffer.iter,ITER_DEST , bvec, i, rreq->len); rreq->submitted = rreq->start + flen; -netfs_read_to_pagecache (rreq,NULL ); +netfs_read_to_pagecache (rreq); ret =netfs_wait_for_read (rreq);if (ret >=0 ) {
@@ -532,7 +553,7 @@ int netfs_read_folio(struct file *file, struct folio *folio)if (ret <0 ) goto discard; -netfs_read_to_pagecache (rreq,NULL ); +netfs_read_to_pagecache (rreq); ret =netfs_wait_for_read (rreq);netfs_put_request (rreq, netfs_rreq_trace_put_return);return ret <0 ? ret :0 ;
@@ -689,7 +710,7 @@ int netfs_write_begin(struct netfs_inode *ctx,if (ret <0 ) goto error_put; -netfs_read_to_pagecache (rreq,NULL ); +netfs_read_to_pagecache (rreq); ret =netfs_wait_for_read (rreq);netfs_put_request (rreq, netfs_rreq_trace_put_return);if (ret <0 )
@@ -754,7 +775,7 @@ int netfs_prefetch_for_write(struct file *file, struct folio *folio,if (ret <0 ) goto error_put; -netfs_read_to_pagecache (rreq,NULL ); +netfs_read_to_pagecache (rreq); ret =netfs_wait_for_read (rreq);netfs_put_request (rreq, netfs_rreq_trace_put_return);return ret <0 ? ret :0 ;
fs/netfs/internal.h +1 -0
@@ -79,6 +79,7 @@ ssize_t netfs_wait_for_read(struct netfs_io_request *rreq);ssize_t netfs_wait_for_write (struct netfs_io_request *rreq);void netfs_wait_for_paused_read (struct netfs_io_request *rreq);void netfs_wait_for_paused_write (struct netfs_io_request *rreq); +void netfs_wait_for_put_ra_refs (struct netfs_io_request *rreq);/* * objects.c
fs/netfs/misc.c +19 -0
@@ -563,3 +563,22 @@ void netfs_wait_for_paused_write(struct netfs_io_request *rreq) {return netfs_wait_for_pause (rreq, netfs_write_collection); } + +/* +* Wait for the readahead-acquired refs to be put. +*/ +void netfs_wait_for_put_ra_refs (struct netfs_io_request *rreq) +{ +DEFINE_WAIT (myself); + +for (;;) { +trace_netfs_rreq (rreq, netfs_rreq_trace_wait_put_ra_refs); +prepare_to_wait (&rreq->waitq, &myself,TASK_UNINTERRUPTIBLE ); +if (!test_bit (NETFS_RREQ_NEED_PUT_RA_REFS , &rreq->flags)) +break ; +schedule (); + } + +trace_netfs_rreq (rreq, netfs_rreq_trace_waited_put_ra_refs); +finish_wait (&rreq->waitq, &myself); +}
fs/netfs/read_collect.c +7 -0
@@ -118,6 +118,13 @@ static void netfs_read_unlock_folios(struct netfs_io_request *rreq, slot =0 ; } +/* We have to wait for readahead refs to have been released before we +* can unlock any folios as the ref-dropper walks i_pages and the only +* thing preventing these folios from being removed is the folio lock. +*/ +if (test_bit (NETFS_RREQ_NEED_PUT_RA_REFS , &rreq->flags)) +netfs_wait_for_put_ra_refs (rreq); +for (;;) {struct folio *folio;unsigned long long fpos, fend;
fs/netfs/read_retry.c +7 -0
@@ -292,6 +292,13 @@ void netfs_unlock_abandoned_read_pages(struct netfs_io_request *rreq) {struct folio_queue *p; +/* We have to wait for readahead refs to have been released before we +* can unlock any folios as the ref-dropper walks i_pages and the only +* thing preventing these folios from being removed is the folio lock. +*/ +if (test_bit (NETFS_RREQ_NEED_PUT_RA_REFS , &rreq->flags)) +netfs_wait_for_put_ra_refs (rreq); +for (p = rreq->buffer.tail; p; p = p->next) {for (int slot =0 ; slot <folioq_count (p); slot++) {struct folio *folio =folioq_folio (p, slot);
fs/netfs/rolling_buffer.c +52 -29
94 lines not on this page · raw message
include/linux/netfs.h +1 -0
@@ -278,6 +278,7 @@ struct netfs_io_request {#define NETFS_RREQ_FOLIO_COPY_TO_CACHE 10/* Copy current folio to cache from read */ #define NETFS_RREQ_UPLOAD_TO_SERVER 11/* Need to write to the server */ #define NETFS_RREQ_USE_IO_ITER 12/* Use ->io_iter rather than ->i_pages */ +#define NETFS_RREQ_NEED_PUT_RA_REFS 17/* Need to put the folio refs RA gave us */ #define NETFS_RREQ_USE_PGPRIV2 31/* [DEPRECATED] Use PG_private_2 to mark * write to cache on read */ const struct netfs_request_ops *netfs_ops;
include/linux/rolling_buffer.h +3 -3
@@ -45,9 +45,9 @@ struct rolling_buffer_snapshot {int rolling_buffer_init (struct rolling_buffer *roll,unsigned int rreq_id,unsigned int direction,gfp_t gfp);int rolling_buffer_make_space (struct rolling_buffer *roll,gfp_t gfp); -ssize_t rolling_buffer_load_from_ra (struct rolling_buffer *roll, -struct readahead_control *ractl, -struct folio_batch *put_batch); +ssize_t rolling_buffer_bulk_load_from_ra (struct rolling_buffer *roll, +struct readahead_control *ractl, +unsigned int rreq_id,gfp_t gfp);ssize_t rolling_buffer_append (struct rolling_buffer *roll,struct folio *folio,unsigned int flags,gfp_t gfp);struct folio_queue *rolling_buffer_delete_spent (struct rolling_buffer *roll);
include/trace/events/netfs.h +3 -0
@@ -59,6 +59,7 @@ EM(netfs_rreq_trace_free, "FREE ") \ EM(netfs_rreq_trace_intr, "INTR ") \ EM(netfs_rreq_trace_ki_complete, "KI-CMPL") \ + EM(netfs_rreq_trace_ra_put_ref, "RA-PUT ") \ EM(netfs_rreq_trace_recollect, "RECLLCT") \ EM(netfs_rreq_trace_redirty, "REDIRTY") \EM (netfs_rreq_trace_resubmit,"RESUBMT" ) \
@@ -70,9 +71,11 @@ EM(netfs_rreq_trace_unpause, "UNPAUSE") \ EM(netfs_rreq_trace_wait_ip, "WAIT-IP") \ EM(netfs_rreq_trace_wait_pause, "--PAUSED--") \ + EM(netfs_rreq_trace_wait_put_ra_refs, "WAIT-P-RA") \ EM(netfs_rreq_trace_wait_quiesce, "WAIT-QUIESCE") \ EM(netfs_rreq_trace_waited_ip, "DONE-IP") \ EM(netfs_rreq_trace_waited_pause, "--UNPAUSED--") \ + EM(netfs_rreq_trace_waited_put_ra_refs, "DONE-P-RA") \ EM(netfs_rreq_trace_waited_quiesce, "DONE-QUIESCE") \ EM(netfs_rreq_trace_wake_ip, "WAKE-IP") \EM (netfs_rreq_trace_wake_queue,"WAKE-Q " ) \
David Howells
maintainerPATCH v2 8/9 netfs: Fix read progress reporting
In reply to David HowellsFor 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.
--- fs/netfs/buffered_read.c | 2 ++ 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, 91 insertions(+), 27 deletions(-)
fs/netfs/buffered_read.c +2 -0
@@ -383,6 +383,7 @@ void netfs_readahead(struct readahead_control *ractl) rreq->submitted = rreq->start + added; rreq->cleaned_to = rreq->start; +netfs_read_set_unlock_at (rreq);netfs_read_to_pagecache (rreq);netfs_maybe_bulk_drop_ra_refs (rreq);
@@ -408,6 +409,7 @@ static int netfs_create_singular_buffer(struct netfs_io_request *rreq, struct foif (added <0 )return added; rreq->submitted = rreq->start + added; + rreq->progress_at = added;return 0 ; }
fs/netfs/internal.h +1 -0
@@ -110,6 +110,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);
fs/netfs/objects.c +20 -12
@@ -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 ||
fs/netfs/read_collect.c +44 -14
121 lines not on this page · raw message
fs/netfs/read_single.c +2 -0
@@ -170,6 +170,8 @@ ssize_t netfs_read_single(struct inode *inode, struct file *file, struct iov_iteif (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;
include/linux/netfs.h +1 -1
@@ -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;
include/trace/events/netfs.h +21 -0
@@ -789,6 +789,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 */
David Howells
maintainerPATCH v2 9/9 cachefiles: Fix potential UAF/KASAN warning
In reply to David HowellsCurrently, 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.
--- fs/cachefiles/xattr.c | 16 ++++++++-------- include/trace/events/cachefiles.h | 19 +++++++++++++++++-- 2 files changed, 25 insertions(+), 10 deletions(-)
fs/cachefiles/xattr.c +8 -8
@@ -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 ) +max (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 *fileint ret = -ESTALE ; tlen =sizeof (struct cachefiles_xattr ) + len; - buf =kmalloc (tlen,GFP_KERNEL ); + buf =kmalloc (sizeof (struct cachefiles_xattr ) +max (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; }
include/trace/events/cachefiles.h +17 -2
@@ -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 2-byte aligned but not 8-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",