[PATCH v2] mm/page-writeback: document folio_mark_dirty() locking more explicitly

Jann Horn <[email protected]>
Newsgroups org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kvack.linux-mm
Message-ID <[email protected]>
We have had bugs where set_page_dirty() was used on a page from GUP without
appropriate locking, leading to UAF, in:

 - KVM, see
   https://lore.kernel.org/r/[email protected]
 - i915, see commit 0d4bbe3d407f ("drm/i915/userptr: Try to acquire the
   page lock around set_page_dirty()").
 - VMCI, see commit 5a16c535409f ("VMCI: Use set_page_dirty_lock() when
   unregistering guest memory")
 - kpc2000 staging driver, see commit b6d13bd9f2c1 ("staging: kpc2000:
   kpc_dma: Convert set_page_dirty() --> set_page_dirty_lock()")

I think set_page_dirty() and folio_mark_dirty() need more explicit
documentation on how they should be used with pages from GUP; so add a
comment on top of set_page_dirty() and make the comment above
folio_mark_dirty() more explicit.

Signed-off-by: Jann Horn <[email protected]>
---
Changes in v2:
- reword commit message to drop references to out-of-tree drivers, and
  instead add more examples of upstream bugs
- Link to v1: https://patch.msgid.link/[email protected]
---
 mm/folio-compat.c   | 1 +
 mm/page-writeback.c | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/mm/folio-compat.c b/mm/folio-compat.c
index a02179a0bded..6212fdd6761a 100644
--- a/mm/folio-compat.c
+++ b/mm/folio-compat.c
@@ -41,6 +41,7 @@ void set_page_writeback(struct page *page)
 }
 EXPORT_SYMBOL(set_page_writeback);
 
+/* Read the comment above folio_mark_dirty() regarding required locks! */
 bool set_page_dirty(struct page *page)
 {
 	return folio_mark_dirty(page_folio(page));
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index e98748112d1e..b0ab687c83be 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2773,6 +2773,11 @@ EXPORT_SYMBOL(folio_redirty_for_writepage);
  * in this folio.  Truncation will block on the page table lock as it
  * unmaps pages before removing the folio from its mapping.
  *
+ * .. DANGER::
+ *    Do not use this on a folio obtained from a function like
+ *    get_user_pages_fast() without holding appropriate locks; you might want to
+ *    use set_page_dirty_lock() or folio_mark_dirty_lock() instead.
+ *
  * Return: True if the folio was newly dirtied, false if it was already dirty.
  */
 bool folio_mark_dirty(struct folio *folio)

---
base-commit: db2ddb87143519e20a95aa36c60b36107b736a58
change-id: 20260810-set-page-dirty-warnings-4000f0015394

Best regards,
--  
Jann Horn <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.