linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Christian Brauner <christian@brauner.io>,
	Steve French <smfrench@gmail.com>,
	Matthew Wilcox <willy@infradead.org>
Cc: David Howells <dhowells@redhat.com>,
	Jeff Layton <jlayton@kernel.org>,
	Gao Xiang <hsiangkao@linux.alibaba.com>,
	Dominique Martinet <asmadeus@codewreck.org>,
	Marc Dionne <marc.dionne@auristor.com>,
	Paulo Alcantara <pc@manguebit.com>,
	Shyam Prasad N <sprasad@microsoft.com>,
	Tom Talpey <tom@talpey.com>,
	Eric Van Hensbergen <ericvh@kernel.org>,
	Ilya Dryomov <idryomov@gmail.com>,
	netfs@lists.linux.dev, linux-afs@lists.infradead.org,
	linux-cifs@vger.kernel.org, linux-nfs@vger.kernel.org,
	ceph-devel@vger.kernel.org, v9fs@lists.linux.dev,
	linux-erofs@lists.ozlabs.org, linux-fsdevel@vger.kernel.org,
	linux-mm@kvack.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	syzbot+af5c06208fa71bf31b16@syzkaller.appspotmail.com,
	Chang Yu <marcus.yu.56@gmail.com>
Subject: [PATCH v5 32/32] netfs: Report on NULL folioq in netfs_writeback_unlock_folios()
Date: Mon, 16 Dec 2024 20:41:22 +0000	[thread overview]
Message-ID: <20241216204124.3752367-33-dhowells@redhat.com> (raw)
In-Reply-To: <20241216204124.3752367-1-dhowells@redhat.com>

It seems that it's possible to get to netfs_writeback_unlock_folios() with
an empty rolling buffer during buffered writes.  This should not be
possible as the rolling buffer is initialised as the write request is set
up and thereafter maintains at least one folio_queue struct therein until
it gets destroyed.  This allows lockless addition and removal of
folio_queue structs in the buffer because, unlike with a ring buffer, the
producer and consumer each only need to look at and alter one pointer into
the buffer.

Now, the rolling buffer is only used for buffered I/O operations as
netfs_collect_write_results() should only call
netfs_writeback_unlock_folios() if the request is of origin type
NETFS_WRITEBACK, NETFS_WRITETHROUGH or NETFS_PGPRIV2_COPY_TO_CACHE.

So it would seem that one of the following occurred: (1) I/O started before
the request was fully initialised, (2) the origin got switched mid-flow or
(3) the request has already been freed and this is a UAF error.  I think
the last is the most likely.

Make netfs_writeback_unlock_folios() report information about the request
and subrequests if folioq is seen to be NULL to try and help debug this,
throw a warning and return.

Note that this does not try to fix the problem.

Reported-by: syzbot+af5c06208fa71bf31b16@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=af5c06208fa71bf31b16
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Chang Yu <marcus.yu.56@gmail.com>
Link: https://lore.kernel.org/r/ZxshMEW4U7MTgQYa@gmail.com/
cc: Jeff Layton <jlayton@kernel.org>
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
---
 fs/netfs/write_collect.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/fs/netfs/write_collect.c b/fs/netfs/write_collect.c
index 1b7f53d01b8d..294f67795f79 100644
--- a/fs/netfs/write_collect.c
+++ b/fs/netfs/write_collect.c
@@ -21,6 +21,34 @@
 #define NEED_RETRY		0x10	/* A front op requests retrying */
 #define SAW_FAILURE		0x20	/* One stream or hit a permanent failure */
 
+static void netfs_dump_request(const struct netfs_io_request *rreq)
+{
+	pr_err("Request R=%08x r=%d fl=%lx or=%x e=%ld\n",
+	       rreq->debug_id, refcount_read(&rreq->ref), rreq->flags,
+	       rreq->origin, rreq->error);
+	pr_err("  st=%llx tsl=%zx/%llx/%llx\n",
+	       rreq->start, rreq->transferred, rreq->submitted, rreq->len);
+	pr_err("  cci=%llx/%llx/%llx\n",
+	       rreq->cleaned_to, rreq->collected_to, atomic64_read(&rreq->issued_to));
+	pr_err("  iw=%pSR\n", rreq->netfs_ops->issue_write);
+	for (int i = 0; i < NR_IO_STREAMS; i++) {
+		const struct netfs_io_subrequest *sreq;
+		const struct netfs_io_stream *s = &rreq->io_streams[i];
+
+		pr_err("  str[%x] s=%x e=%d acnf=%u,%u,%u,%u\n",
+		       s->stream_nr, s->source, s->error,
+		       s->avail, s->active, s->need_retry, s->failed);
+		pr_err("  str[%x] ct=%llx t=%zx\n",
+		       s->stream_nr, s->collected_to, s->transferred);
+		list_for_each_entry(sreq, &s->subrequests, rreq_link) {
+			pr_err("  sreq[%x:%x] sc=%u s=%llx t=%zx/%zx r=%d f=%lx\n",
+			       sreq->stream_nr, sreq->debug_index, sreq->source,
+			       sreq->start, sreq->transferred, sreq->len,
+			       refcount_read(&sreq->ref), sreq->flags);
+		}
+	}
+}
+
 /*
  * Successful completion of write of a folio to the server and/or cache.  Note
  * that we are not allowed to lock the folio here on pain of deadlocking with
@@ -87,6 +115,12 @@ static void netfs_writeback_unlock_folios(struct netfs_io_request *wreq,
 	unsigned long long collected_to = wreq->collected_to;
 	unsigned int slot = wreq->buffer.first_tail_slot;
 
+	if (WARN_ON_ONCE(!folioq)) {
+		pr_err("[!] Writeback unlock found empty rolling buffer!\n");
+		netfs_dump_request(wreq);
+		return;
+	}
+
 	if (wreq->origin == NETFS_PGPRIV2_COPY_TO_CACHE) {
 		if (netfs_pgpriv2_unlock_copied_folios(wreq))
 			*notes |= MADE_PROGRESS;



      parent reply	other threads:[~2024-12-16 20:45 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-16 20:40 [PATCH v5 00/32] netfs: Read performance improvements and "single-blob" support David Howells
2024-12-16 20:40 ` [PATCH v5 01/32] netfs: Clean up some whitespace in trace header David Howells
2024-12-16 20:40 ` [PATCH v5 02/32] cachefiles: " David Howells
2024-12-16 20:40 ` [PATCH v5 03/32] netfs: Use a folio_queue allocation and free functions David Howells
2024-12-16 20:40 ` [PATCH v5 04/32] netfs: Add a tracepoint to log the lifespan of folio_queue structs David Howells
2024-12-16 20:40 ` [PATCH v5 05/32] netfs: Abstract out a rolling folio buffer implementation David Howells
2024-12-16 20:40 ` [PATCH v5 06/32] netfs: Make netfs_advance_write() return size_t David Howells
2024-12-16 20:40 ` [PATCH v5 07/32] netfs: Split retry code out of fs/netfs/write_collect.c David Howells
2024-12-16 20:40 ` [PATCH v5 08/32] netfs: Drop the error arg from netfs_read_subreq_terminated() David Howells
2024-12-16 20:40 ` [PATCH v5 09/32] netfs: Drop the was_async " David Howells
2024-12-16 20:41 ` [PATCH v5 10/32] netfs: Don't use bh spinlock David Howells
2024-12-16 20:41 ` [PATCH v5 11/32] afs: Don't use mutex for I/O operation lock David Howells
2024-12-16 20:41 ` [PATCH v5 12/32] afs: Fix EEXIST error returned from afs_rmdir() to be ENOTEMPTY David Howells
2024-12-16 20:41 ` [PATCH v5 13/32] afs: Fix directory format encoding struct David Howells
2024-12-16 20:41 ` [PATCH v5 14/32] netfs: Remove some extraneous directory invalidations David Howells
2024-12-16 20:41 ` [PATCH v5 15/32] cachefiles: Add some subrequest tracepoints David Howells
2024-12-16 20:41 ` [PATCH v5 16/32] cachefiles: Add auxiliary data trace David Howells
2024-12-16 20:41 ` [PATCH v5 17/32] afs: Add more tracepoints to do with tracking validity David Howells
2024-12-16 20:41 ` [PATCH v5 18/32] netfs: Add functions to build/clean a buffer in a folio_queue David Howells
2024-12-16 20:41 ` [PATCH v5 19/32] netfs: Add support for caching single monolithic objects such as AFS dirs David Howells
2024-12-16 20:41 ` [PATCH v5 20/32] afs: Make afs_init_request() get a key if not given a file David Howells
2024-12-16 20:41 ` [PATCH v5 21/32] afs: Use netfslib for directories David Howells
2024-12-16 20:41 ` [PATCH v5 22/32] afs: Use netfslib for symlinks, allowing them to be cached David Howells
2024-12-16 20:41 ` [PATCH v5 23/32] afs: Eliminate afs_read David Howells
2024-12-16 20:41 ` [PATCH v5 24/32] afs: Fix cleanup of immediately failed async calls David Howells
2024-12-16 20:41 ` [PATCH v5 25/32] afs: Make {Y,}FS.FetchData an asynchronous operation David Howells
2024-12-16 20:41 ` [PATCH v5 26/32] Display waited-on page index after 1min of waiting David Howells
2024-12-16 20:41 ` [PATCH v5 27/32] netfs: Change the read result collector to only use one work item David Howells
2025-01-24 17:59   ` Ihor Solodrai
2025-01-24 18:21     ` Marc Dionne
2025-01-24 18:46       ` Ihor Solodrai
2025-01-24 19:07         ` Marc Dionne
2025-01-24 19:54           ` Ihor Solodrai
2024-12-16 20:41 ` [PATCH v5 28/32] afs: Make afs_mkdir() locally initialise a new directory's content David Howells
2024-12-16 20:41 ` [PATCH v5 29/32] afs: Use the contained hashtable to search a directory David Howells
2024-12-16 20:41 ` [PATCH v5 30/32] afs: Locally initialise the contents of a new symlink on creation David Howells
2024-12-16 20:41 ` [PATCH v5 31/32] afs: Add a tracepoint for afs_read_receive() David Howells
2024-12-16 20:41 ` David Howells [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241216204124.3752367-33-dhowells@redhat.com \
    --to=dhowells@redhat.com \
    --cc=asmadeus@codewreck.org \
    --cc=ceph-devel@vger.kernel.org \
    --cc=christian@brauner.io \
    --cc=ericvh@kernel.org \
    --cc=hsiangkao@linux.alibaba.com \
    --cc=idryomov@gmail.com \
    --cc=jlayton@kernel.org \
    --cc=linux-afs@lists.infradead.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-erofs@lists.ozlabs.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=marc.dionne@auristor.com \
    --cc=marcus.yu.56@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=netfs@lists.linux.dev \
    --cc=pc@manguebit.com \
    --cc=smfrench@gmail.com \
    --cc=sprasad@microsoft.com \
    --cc=syzbot+af5c06208fa71bf31b16@syzkaller.appspotmail.com \
    --cc=tom@talpey.com \
    --cc=v9fs@lists.linux.dev \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox