All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: SHAURYA RANE <ssrane_b23@ee.vjti.ac.in>,
	akpm@linux-foundation.org, shakeel.butt@linux.dev,
	eddyz87@gmail.com, andrii@kernel.org, ast@kernel.org,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org,
	linux-kernel-mentees@lists.linux.dev, skhan@linuxfoundation.org,
	david.hunter.linux@gmail.com, khalid@kernel.org,
	syzbot+09b7d050e4806540153d@syzkaller.appspotmail.com
Subject: Re: [PATCH] mm/filemap: fix NULL pointer dereference in do_read_cache_folio()
Date: Mon, 17 Nov 2025 18:03:40 +0000	[thread overview]
Message-ID: <aRtjfN7sC6_Bv4bx@casper.infradead.org> (raw)
In-Reply-To: <20251117164155.GB196362@frogsfrogsfrogs>

On Mon, Nov 17, 2025 at 08:41:55AM -0800, Darrick J. Wong wrote:
> I wondered why this whole thing opencodes kernel_read, but then I
> noticed zero fstests for it and decid*******************************
> *****.

I wondered the same thing!  And the answer is that it's special BPF
stuff:

        /* if sleeping is allowed, wait for the page, if necessary */
        if (r->may_fault && (IS_ERR(r->folio) || !folio_test_uptodate(r->folio))) {
                filemap_invalidate_lock_shared(r->file->f_mapping);
                r->folio = read_cache_folio(r->file->f_mapping, file_off >> PAGE_SHIFT,
                                            NULL, r->file);
                filemap_invalidate_unlock_shared(r->file->f_mapping);
        }

if 'may_fault' (a misnomer since it really means "may sleep"), then we
essentially do kernel_read().

Now, maybe the right thing to do here is rip out almost all of
lib/buildid.c and replace it with an iocb with IOCB_NOWAIT set (or not).
I was hesitant to suggest this earlier as it's a bit of a big ask of
someone who was just trying to submit a one-line change.  But now that
"it's also shmem" has entered the picture, I'm leaning more towards this
approach anyway.

Looking at it though, it's a bit weird that we don't have a
kiocb_read().  It feels like __kernel_read() needs to be split into
half like:

diff --git a/fs/read_write.c b/fs/read_write.c
index 833bae068770..a3bf962836a7 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -503,14 +503,29 @@ static int warn_unsupported(struct file *file, const char *op)
 	return -EINVAL;
 }
 
-ssize_t __kernel_read(struct file *file, void *buf, size_t count, loff_t *pos)
+ssize_t kiocb_read(struct kiocb *iocb, void *buf, size_t count)
 {
+	struct file *file = iocb->ki_filp;
 	struct kvec iov = {
 		.iov_base	= buf,
 		.iov_len	= min_t(size_t, count, MAX_RW_COUNT),
 	};
-	struct kiocb kiocb;
 	struct iov_iter iter;
+	int ret;
+
+	iov_iter_kvec(&iter, ITER_DEST, &iov, 1, iov.iov_len);
+	ret = file->f_op->read_iter(iocb, &iter);
+	if (ret > 0) {
+		fsnotify_access(file);
+		add_rchar(current, ret);
+	}
+	inc_syscr(current);
+	return ret;
+}
+
+ssize_t __kernel_read(struct file *file, void *buf, size_t count, loff_t *pos)
+{
+	struct kiocb kiocb;
 	ssize_t ret;
 
 	if (WARN_ON_ONCE(!(file->f_mode & FMODE_READ)))
@@ -526,15 +541,9 @@ ssize_t __kernel_read(struct file *file, void *buf, size_t count, loff_t *pos)
 
 	init_sync_kiocb(&kiocb, file);
 	kiocb.ki_pos = pos ? *pos : 0;
-	iov_iter_kvec(&iter, ITER_DEST, &iov, 1, iov.iov_len);
-	ret = file->f_op->read_iter(&kiocb, &iter);
-	if (ret > 0) {
-		if (pos)
-			*pos = kiocb.ki_pos;
-		fsnotify_access(file);
-		add_rchar(current, ret);
-	}
-	inc_syscr(current);
+	ret = kiocb_read(&kiocb, buf, count);
+	if (pos && ret > 0)
+		*pos = kiocb.ki_pos;
 	return ret;
 }
 

  reply	other threads:[~2025-11-17 18:03 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-14 19:37 [PATCH] mm/filemap: fix NULL pointer dereference in do_read_cache_folio() ssrane_b23
2025-11-14 20:44 ` Matthew Wilcox
2025-11-16  5:42   ` [PATCH v2] " ssrane_b23
2025-11-16  5:43   ` [PATCH] " SHAURYA RANE
2025-11-16 22:32     ` Matthew Wilcox
2025-11-17 14:10       ` Shaurya Rane
2025-11-17 18:42         ` Andrii Nakryiko
2025-11-17 16:41       ` Darrick J. Wong
2025-11-17 18:03         ` Matthew Wilcox [this message]
2025-11-17 18:45           ` Andrii Nakryiko
2025-11-18 13:03             ` Christoph Hellwig
2025-11-18 15:37               ` Matthew Wilcox
2025-11-18 16:12                 ` Darrick J. Wong
2025-11-18 19:38                   ` Andrii Nakryiko
2025-11-19  5:52                     ` Christoph Hellwig
2025-11-19  6:29                     ` Darrick J. Wong
2025-11-18 19:27                 ` Andrii Nakryiko
2025-11-19  5:50                   ` Christoph Hellwig
2025-11-19 17:12                     ` Andrii Nakryiko
2025-12-17  7:33                   ` Shakeel Butt
2025-11-18  5:05       ` Christoph Hellwig
2025-11-18 12:51         ` Matthew Wilcox
2025-11-18 12:56           ` Christoph Hellwig

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=aRtjfN7sC6_Bv4bx@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=david.hunter.linux@gmail.com \
    --cc=djwong@kernel.org \
    --cc=eddyz87@gmail.com \
    --cc=khalid@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel-mentees@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=shakeel.butt@linux.dev \
    --cc=skhan@linuxfoundation.org \
    --cc=ssrane_b23@ee.vjti.ac.in \
    --cc=syzbot+09b7d050e4806540153d@syzkaller.appspotmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.