linux-kernel - Re: [REGRESSION] 9pfs issues on 6.12-rc1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3327438.1729678025@warthog.procyon.org.uk>
Date: Wed, 23 Oct 2024 11:07:05 +0100
From: David Howells <dhowells@...hat.com>
To: Antony Antony <antony@...nome.org>
Cc: dhowells@...hat.com, Christian Brauner <brauner@...nel.org>,
    Eric Van Hensbergen <ericvh@...nel.org>,
    Latchesar Ionkov <lucho@...kov.net>,
    Dominique Martinet <asmadeus@...ewreck.org>,
    Christian Schoenebeck <linux_oss@...debyte.com>,
    Sedat Dilek <sedat.dilek@...il.com>,
    Maximilian Bosch <maximilian@...sch.me>, regressions@...ts.linux.dev,
    v9fs@...ts.linux.dev, netfs@...ts.linux.dev,
    linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [REGRESSION] 9pfs issues on 6.12-rc1

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@...hat.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@...nome.org>
    Closes: https://lore.kernel.org/r/ZxFQw4OI9rrc7UYc@Antony2201.local/
    Signed-off-by: David Howells <dhowells@...hat.com>
    cc: Eric Van Hensbergen <ericvh@...nel.org>
    cc: Latchesar Ionkov <lucho@...kov.net>
    cc: Dominique Martinet <asmadeus@...ewreck.org>
    cc: Christian Schoenebeck <linux_oss@...debyte.com>
    cc: v9fs@...ts.linux.dev
    cc: netfs@...ts.linux.dev
    cc: linux-fsdevel@...r.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;