Syzkaller reports warning in ext4_set_page_dirty() in 5.10 and 5.15 stable releases. It happens because invalidate_inode_page() frees pages that are needed for the system. To fix this we need to add additional checks to the function. page_mapped() checks if a page exists in the page tables, but this is not enough. The page can be used in other places: https://elixir.bootlin.com/linux/v6.8-rc1/source/include/linux/page_ref.h#L7...
Kernel outputs an error line related to direct I/O: https://syzkaller.appspot.com/text?tag=CrashLog&x=14ab52dac80000
The problem can be fixed in 5.10 and 5.15 stable releases by the following patch.
The patch replaces page_mapped() call with check that finds additional references to the page excluding page cache and filesystem private data. If additional references exist, the page cannot be freed.
This version does not include the first patch from the first version. The problem can be fixed without it.
Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
Link: https://syzkaller.appspot.com/bug?extid=02f21431b65c214aa1d6
Previous discussion: https://lore.kernel.org/all/20240125130947.600632-1-r.smirnov@omp.ru/T/
Matthew Wilcox (Oracle) (1): mm/truncate: Replace page_mapped() call in invalidate_inode_page()
mm/truncate.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
From: "Matthew Wilcox (Oracle)" willy@infradead.org
commit e41c81d0d30e1a6ebf408feaf561f80cac4457dc upstream.
folio_mapped() is expensive because it has to check each page's mapcount field. A cheaper check is whether there are any extra references to the page, other than the one we own, one from the page private data and the ones held by the page cache.
The call to remove_mapping() will fail in any case if it cannot freeze the refcount, but failing here avoids cycling the i_pages spinlock.
Signed-off-by: Matthew Wilcox (Oracle) willy@infradead.org Reviewed-by: Miaohe Lin linmiaohe@huawei.com [Roman: replaced folio_ref_count() call with page_ref_count(), folio_nr_pages() call with compound_nr(), and folio_has_private() call with page_has_private()] Signed-off-by: Roman Smirnov r.smirnov@omp.ru --- mm/truncate.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/mm/truncate.c b/mm/truncate.c index 8914ca4ce4b1..989bc7785d55 100644 --- a/mm/truncate.c +++ b/mm/truncate.c @@ -256,7 +256,9 @@ int invalidate_inode_page(struct page *page) return 0; if (PageDirty(page) || PageWriteback(page)) return 0; - if (page_mapped(page)) + /* The refcount will be elevated if the page is used by the system */ + if (page_ref_count(page) > + compound_nr(page) + page_has_private(page) + 1) return 0; return invalidate_complete_page(mapping, page); }
On Tue, 13 Feb, 2024 14:09:33 +0000, Roman Smirnov wrote:
Syzkaller reports warning in ext4_set_page_dirty() in 5.10 and 5.15 stable releases. It happens because invalidate_inode_page() frees pages that are needed for the system. To fix this we need to add additional checks to the function. page_mapped() checks if a page exists in the page tables, but this is not enough. The page can be used in other places: https://elixir.bootlin.com/linux/v6.8-rc1/source/include/linux/page_ref.h#L7...
Kernel outputs an error line related to direct I/O: https://syzkaller.appspot.com/text?tag=CrashLog&x=14ab52dac80000
The problem can be fixed in 5.10 and 5.15 stable releases by the following patch.
The patch replaces page_mapped() call with check that finds additional references to the page excluding page cache and filesystem private data. If additional references exist, the page cannot be freed.
This version does not include the first patch from the first version. The problem can be fixed without it.
Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
Link: https://syzkaller.appspot.com/bug?extid=02f21431b65c214aa1d6
Previous discussion: https://lore.kernel.org/all/20240125130947.600632-1-r.smirnov@omp.ru/T/
Matthew Wilcox (Oracle) (1): mm/truncate: Replace page_mapped() call in invalidate_inode_page()
mm/truncate.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Hello.
Sorry to bother you, do you have any comments on the patch?
linux-stable-mirror@lists.linaro.org
-
Roman Smirnov