From: Ketan Kishore <ketan.kishore@oss.qualcomm.com>
To: Matthew Wilcox <willy@infradead.org>,
"David Hildenbrand (Arm)" <david@kernel.org>
Cc: Luiz Capitulino <luizcap@redhat.com>,
Andrew Morton <akpm@linux-foundation.org>,
Vlastimil Babka <vbabka@kernel.org>,
Suren Baghdasaryan <surenb@google.com>,
Michal Hocko <mhocko@suse.com>,
Brendan Jackman <jackmanb@google.com>,
Johannes Weiner <hannes@cmpxchg.org>, Zi Yan <ziy@nvidia.com>,
kernel@oss.qualcomm.com, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm: page_ext: validate section in for_each_page_ext iterator
Date: Sat, 20 Jun 2026 02:14:06 +0530 [thread overview]
Message-ID: <2c77c4f6-cae1-4bd4-aa7b-82da96334d6f@oss.qualcomm.com> (raw)
In-Reply-To: <ajVGQg0hJCUThz5Q@casper.infradead.org>
On 6/19/2026 7:08 PM, Matthew Wilcox wrote:
> On Fri, Jun 19, 2026 at 10:00:59AM +0200, David Hildenbrand (Arm) wrote:
>> Likely we should just handle that in the iterator, looking up page-ext for something
>> that doesn't exist is weird.
>
> Yes, but I prefer to pass 'max' to the _next step explicitly, rather
> than put it in the iterator struct. Untested patch attached.
>
> The other way to do it is s/index/remaining/ and count it down rather
> than upwards. But that assumes none of the iterator users look at
> iter->index, and I don't have the patience to check that.
>
> I find it weird that somebody botheresd to add kernel-doc for
> page_ext_iter_begin/next. They're internal implementation and really
> shouldn't have public documentation.
>
> diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
> index 61e876e255e8..4f7d7a8709de 100644
> --- a/include/linux/page_ext.h
> +++ b/include/linux/page_ext.h
> @@ -120,14 +120,18 @@ struct page_ext_iter {
> * page_ext_iter_begin() - Prepare for iterating through page extensions.
> * @iter: page extension iterator.
> * @pfn: PFN of the page we're interested in.
> + * @count: maximum number of page extensions to return.
> *
> * Must be called with RCU read lock taken.
> *
> * Return: NULL if no page_ext exists for this page.
> */
> static inline struct page_ext *page_ext_iter_begin(struct page_ext_iter *iter,
> - unsigned long pfn)
> + unsigned long pfn, unsigned long count)
> {
> + if (count == 0)
> + return NULL;
> +
> iter->index = 0;
> iter->start_pfn = pfn;
> iter->page_ext = page_ext_lookup(pfn);
> @@ -138,19 +142,22 @@ static inline struct page_ext *page_ext_iter_begin(struct page_ext_iter *iter,
> /**
> * page_ext_iter_next() - Get next page extension
> * @iter: page extension iterator.
> + * @count: maximum number of page extensions to return.
> *
> * Must be called with RCU read lock taken.
> *
> * Return: NULL if no next page_ext exists.
> */
> -static inline struct page_ext *page_ext_iter_next(struct page_ext_iter *iter)
> +static inline struct page_ext *page_ext_iter_next(struct page_ext_iter *iter,
> + unsigned long count)
> {
> unsigned long pfn;
>
> if (WARN_ON_ONCE(!iter->page_ext))
> return NULL;
>
> - iter->index++;
> + if (iter->index++ >= count)
> + return NULL;
> pfn = iter->start_pfn + iter->index;
>
> if (page_ext_iter_next_fast_possible(pfn))
> @@ -183,9 +190,9 @@ static inline struct page_ext *page_ext_iter_get(const struct page_ext_iter *ite
> * IMPORTANT: must be called with RCU read lock taken.
> */
> #define for_each_page_ext(__page, __pgcount, __page_ext, __iter) \
> - for (__page_ext = page_ext_iter_begin(&__iter, page_to_pfn(__page));\
> - __page_ext && __iter.index < __pgcount; \
> - __page_ext = page_ext_iter_next(&__iter))
> + for (__page_ext = page_ext_iter_begin(&__iter, page_to_pfn(__page), __pgcount); \
> + __page_ext; \
> + __page_ext = page_ext_iter_next(&__iter, __pgcount))
>
> #else /* !CONFIG_PAGE_EXTENSION */
> struct page_ext;
Thanks Matthew for the suggestions and the proposed alternate fix.
I have tested both your and David's patch, the reported issue is not
observed anymore with both the suggested patches.
Since, in the reply from David on your suggested fix he agrees with the
changes as they are fixing the issue in a similar way.
I'll go ahead and post the v2 with your suggestions.
Thanks & Regards,
Ketan
next prev parent reply other threads:[~2026-06-19 20:44 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-17 15:09 Ketan
2026-06-17 15:52 ` Zi Yan
2026-06-18 9:04 ` David Hildenbrand (Arm)
2026-06-19 2:38 ` Luiz Capitulino
2026-06-19 8:00 ` David Hildenbrand (Arm)
2026-06-19 13:38 ` Matthew Wilcox
2026-06-19 13:45 ` David Hildenbrand (Arm)
2026-06-19 20:44 ` Ketan Kishore [this message]
2026-06-19 20:36 ` Ketan Kishore
2026-06-19 20:31 ` Ketan Kishore
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=2c77c4f6-cae1-4bd4-aa7b-82da96334d6f@oss.qualcomm.com \
--to=ketan.kishore@oss.qualcomm.com \
--cc=akpm@linux-foundation.org \
--cc=david@kernel.org \
--cc=hannes@cmpxchg.org \
--cc=jackmanb@google.com \
--cc=kernel@oss.qualcomm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=luizcap@redhat.com \
--cc=mhocko@suse.com \
--cc=surenb@google.com \
--cc=vbabka@kernel.org \
--cc=willy@infradead.org \
--cc=ziy@nvidia.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 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®