mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Antony Antony <antony@phenome.org>
Cc: dhowells@redhat.com, Christian Brauner <brauner@kernel.org>,
	Eric Van Hensbergen <ericvh@kernel.org>,
	Latchesar Ionkov <lucho@ionkov.net>,
	Dominique Martinet <asmadeus@codewreck.org>,
	Christian Schoenebeck <linux_oss@crudebyte.com>,
	Sedat Dilek <sedat.dilek@gmail.com>,
	Maximilian Bosch <maximilian@mbosch.me>,
	regressions@lists.linux.dev, v9fs@lists.linux.dev,
	netfs@lists.linux.dev, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [REGRESSION] 9pfs issues on 6.12-rc1
Date: Wed, 23 Oct 2024 11:07:05 +0100	[thread overview]
Message-ID: <3327438.1729678025@warthog.procyon.org.uk> (raw)
In-Reply-To: <ZxFQw4OI9rrc7UYc@Antony2201.local>

Hi Antony,

I think the attached should fix it properly rather than working around it as
the previous patch did.  If you could give it a whirl?

Thanks,
David
---
commit 68dddbfdf45e8f176cc8556a3db69af24dfb8519
Author: David Howells <dhowells@redhat.com>
Date:   Wed Oct 23 10:24:12 2024 +0100

    iov_iter: Fix iov_iter_get_pages*() for folio_queue
    
    p9_get_mapped_pages() uses iov_iter_get_pages_alloc2() to extract pages
    from an iterator when performing a zero-copy request and under some
    circumstances, this crashes with odd page errors[1], for example, I see:
    
        page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0xbcf0
        flags: 0x2000000000000000(zone=1)
        ...
        page dumped because: VM_BUG_ON_FOLIO(((unsigned int) folio_ref_count(folio) + 127u <= 127u))
        ------------[ cut here ]------------
        kernel BUG at include/linux/mm.h:1444!
    
    This is because, unlike in iov_iter_extract_folioq_pages(), the
    iter_folioq_get_pages() helper function doesn't skip the current folio when
    iov_offset points to the end of it, but rather extracts the next page
    beyond the end of the folio and adds it to the list.  Reading will then
    clobber the contents of this page, leading to system corruption, and if the
    page is not in use, put_page() may try to clean up the unused page.
    
    This can be worked around by copying the iterator before each extraction[2]
    and using iov_iter_advance() on the original as the advance function steps
    over the page we're at the end of.
    
    Fix this by skipping the page extraction if we're at the end of the folio.
    
    This was reproduced in the ktest environment[3] by forcing 9p to use the
    fscache caching mode and then reading a file through 9p.
    
    Fixes: db0aa2e9566f ("mm: Define struct folio_queue and ITER_FOLIOQ to handle a sequence of folios")
    Reported-by: Antony Antony <antony@phenome.org>
    Closes: https://lore.kernel.org/r/ZxFQw4OI9rrc7UYc@Antony2201.local/
    Signed-off-by: David Howells <dhowells@redhat.com>
    cc: Eric Van Hensbergen <ericvh@kernel.org>
    cc: Latchesar Ionkov <lucho@ionkov.net>
    cc: Dominique Martinet <asmadeus@codewreck.org>
    cc: Christian Schoenebeck <linux_oss@crudebyte.com>
    cc: v9fs@lists.linux.dev
    cc: netfs@lists.linux.dev
    cc: linux-fsdevel@vger.kernel.org
    Link: https://lore.kernel.org/r/ZxFEi1Tod43pD6JC@moon.secunet.de/ [1]
    Link: https://lore.kernel.org/r/2299159.1729543103@warthog.procyon.org.uk/ [2]
    Link: https://github.com/koverstreet/ktest.git [3]

diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 1abb32c0da50..cc4b5541eef8 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1021,15 +1021,18 @@ static ssize_t iter_folioq_get_pages(struct iov_iter *iter,
 		size_t offset = iov_offset, fsize = folioq_folio_size(folioq, slot);
 		size_t part = PAGE_SIZE - offset % PAGE_SIZE;
 
-		part = umin(part, umin(maxsize - extracted, fsize - offset));
-		count -= part;
-		iov_offset += part;
-		extracted += part;
-
-		*pages = folio_page(folio, offset / PAGE_SIZE);
-		get_page(*pages);
-		pages++;
-		maxpages--;
+		if (offset < fsize) {
+			part = umin(part, umin(maxsize - extracted, fsize - offset));
+			count -= part;
+			iov_offset += part;
+			extracted += part;
+
+			*pages = folio_page(folio, offset / PAGE_SIZE);
+			get_page(*pages);
+			pages++;
+			maxpages--;
+		}
+
 		if (maxpages == 0 || extracted >= maxsize)
 			break;
 

  parent reply	other threads:[~2024-10-23 10:07 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <D4LHHUNLG79Y.12PI0X6BEHRHW@mbosch.me>
2024-10-02 17:31 ` Linux regression tracking (Thorsten Leemhuis)
2024-10-02 21:48   ` Maximilian Bosch
2024-10-03  1:12     ` Sedat Dilek
2024-10-17 18:00       ` Antony Antony
2024-10-21 13:23         ` Christian Brauner
2024-10-23 18:35         ` Maximilian Bosch
2024-10-21 14:12       ` David Howells
2024-10-21 15:33         ` Antony Antony
2024-10-21 14:45       ` David Howells
2024-10-21 15:53         ` Antony Antony
2024-10-21 19:48         ` David Howells
2025-08-10  5:10         ` Arnout Engelen
2024-10-21 20:38       ` [PATCH] 9p: Don't revert the I/O iterator after reading David Howells
2024-10-21 23:53         ` Antony Antony
2024-10-22  8:56         ` Christian Brauner
2024-10-23 10:07       ` David Howells [this message]
2024-10-23 19:38         ` [REGRESSION] 9pfs issues on 6.12-rc1 Antony Antony
2025-06-12 22:24           ` Ryan Lahfa
2025-06-27  5:44             ` Christian Theune
2025-06-27  6:44               ` Dominique Martinet
2025-06-27  8:19                 ` Christian Theune
2025-08-10 17:57             ` Arnout Engelen
2025-08-11  0:57               ` asmadeus
2025-08-11  7:43                 ` Dominique Martinet
2025-08-11 12:43                   ` Arnout Engelen
2025-06-27 10:00           ` David Howells
2025-06-27 10:33             ` Ryan Lahfa

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=3327438.1729678025@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=antony@phenome.org \
    --cc=asmadeus@codewreck.org \
    --cc=brauner@kernel.org \
    --cc=ericvh@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux_oss@crudebyte.com \
    --cc=lucho@ionkov.net \
    --cc=maximilian@mbosch.me \
    --cc=netfs@lists.linux.dev \
    --cc=regressions@lists.linux.dev \
    --cc=sedat.dilek@gmail.com \
    --cc=v9fs@lists.linux.dev \
    /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

all inboxes | Powered by JetHome®