Linux filesystem development
 help / color / mirror / Atom feed
* [PATCH] fs/proc/task_mmu: check cur_buf for NULL
@ 2025-09-19 14:21 Jakub Acs
  2025-09-19 16:22 ` Andrei Vagin
  0 siblings, 1 reply; 5+ messages in thread
From: Jakub Acs @ 2025-09-19 14:21 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: acsjakub, Andrew Morton, David Hildenbrand, Vlastimil Babka,
	Lorenzo Stoakes, Jinjiang Tu, Suren Baghdasaryan, Penglei Jiang,
	Mark Brown, Baolin Wang, Ryan Roberts, Andrei Vagin,
	Michał Mirosław, Stephen Rothwell, Muhammad Usama Anjum,
	stable

When PAGEMAP_SCAN ioctl invoked with vec_len = 0 reaches
pagemap_scan_backout_range(), kernel panics with null-ptr-deref:

[   44.936808] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN NOPTI
[   44.937797] KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
[   44.938391] CPU: 1 UID: 0 PID: 2480 Comm: reproducer Not tainted 6.17.0-rc6 #22 PREEMPT(none)
[   44.939062] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
[   44.939935] RIP: 0010:pagemap_scan_thp_entry.isra.0+0x741/0xa80

<snip registers, unreliable trace>

[   44.946828] Call Trace:
[   44.947030]  <TASK>
[   44.949219]  pagemap_scan_pmd_entry+0xec/0xfa0
[   44.952593]  walk_pmd_range.isra.0+0x302/0x910
[   44.954069]  walk_pud_range.isra.0+0x419/0x790
[   44.954427]  walk_p4d_range+0x41e/0x620
[   44.954743]  walk_pgd_range+0x31e/0x630
[   44.955057]  __walk_page_range+0x160/0x670
[   44.956883]  walk_page_range_mm+0x408/0x980
[   44.958677]  walk_page_range+0x66/0x90
[   44.958984]  do_pagemap_scan+0x28d/0x9c0
[   44.961833]  do_pagemap_cmd+0x59/0x80
[   44.962484]  __x64_sys_ioctl+0x18d/0x210
[   44.962804]  do_syscall_64+0x5b/0x290
[   44.963111]  entry_SYSCALL_64_after_hwframe+0x76/0x7e

vec_len = 0 in pagemap_scan_init_bounce_buffer() means no buffers are
allocated and p->vec_buf remains set to NULL.

This breaks an assumption made later in pagemap_scan_backout_range(),
that page_region is always allocated for p->vec_buf_index.

Fix it by explicitly checking cur_buf for NULL before dereferencing.

Other sites that might run into same deref-issue are already (directly
or transitively) protected by checking p->vec_buf.

Note:
From PAGEMAP_SCAN man page, it seems vec_len = 0 is valid when no output
is requested and it's only the side effects caller is interested in,
hence it passes check in pagemap_scan_get_args().

This issue was found by syzkaller.

Fixes: 52526ca7fdb9 ("fs/proc/task_mmu: implement IOCTL to get and optionally clear info about PTEs")
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Jinjiang Tu <tujinjiang@huawei.com>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Penglei Jiang <superman.xpt@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Andrei Vagin <avagin@gmail.com>
Cc: "Michał Mirosław" <mirq-linux@rere.qmqm.pl>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
linux-kernel@vger.kernel.org
linux-fsdevel@vger.kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Jakub Acs <acsjakub@amazon.de>

---
 fs/proc/task_mmu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 29cca0e6d0ff..8c10a8135e74 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -2417,6 +2417,9 @@ static void pagemap_scan_backout_range(struct pagemap_scan_private *p,
 {
 	struct page_region *cur_buf = &p->vec_buf[p->vec_buf_index];
 
+	if (!cur_buf)
+		return;
+
 	if (cur_buf->start != addr)
 		cur_buf->end = addr;
 	else
-- 
2.47.3




Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christian Schlaeger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] fs/proc/task_mmu: check cur_buf for NULL
  2025-09-19 14:21 [PATCH] fs/proc/task_mmu: check cur_buf for NULL Jakub Acs
@ 2025-09-19 16:22 ` Andrei Vagin
  2025-09-22  7:24   ` Jakub Acs
  0 siblings, 1 reply; 5+ messages in thread
From: Andrei Vagin @ 2025-09-19 16:22 UTC (permalink / raw)
  To: Jakub Acs
  Cc: linux-fsdevel, Andrew Morton, David Hildenbrand, Vlastimil Babka,
	Lorenzo Stoakes, Jinjiang Tu, Suren Baghdasaryan, Penglei Jiang,
	Mark Brown, Baolin Wang, Ryan Roberts, Michał Mirosław,
	Stephen Rothwell, Muhammad Usama Anjum, stable

On Fri, Sep 19, 2025 at 7:21 AM Jakub Acs <acsjakub@amazon.de> wrote:
>
> When PAGEMAP_SCAN ioctl invoked with vec_len = 0 reaches
> pagemap_scan_backout_range(), kernel panics with null-ptr-deref:
>
> [   44.936808] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN NOPTI
> [   44.937797] KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> [   44.938391] CPU: 1 UID: 0 PID: 2480 Comm: reproducer Not tainted 6.17.0-rc6 #22 PREEMPT(none)
> [   44.939062] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
> [   44.939935] RIP: 0010:pagemap_scan_thp_entry.isra.0+0x741/0xa80
>
> <snip registers, unreliable trace>
>
> [   44.946828] Call Trace:
> [   44.947030]  <TASK>
> [   44.949219]  pagemap_scan_pmd_entry+0xec/0xfa0
> [   44.952593]  walk_pmd_range.isra.0+0x302/0x910
> [   44.954069]  walk_pud_range.isra.0+0x419/0x790
> [   44.954427]  walk_p4d_range+0x41e/0x620
> [   44.954743]  walk_pgd_range+0x31e/0x630
> [   44.955057]  __walk_page_range+0x160/0x670
> [   44.956883]  walk_page_range_mm+0x408/0x980
> [   44.958677]  walk_page_range+0x66/0x90
> [   44.958984]  do_pagemap_scan+0x28d/0x9c0
> [   44.961833]  do_pagemap_cmd+0x59/0x80
> [   44.962484]  __x64_sys_ioctl+0x18d/0x210
> [   44.962804]  do_syscall_64+0x5b/0x290
> [   44.963111]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> vec_len = 0 in pagemap_scan_init_bounce_buffer() means no buffers are
> allocated and p->vec_buf remains set to NULL.
>
> This breaks an assumption made later in pagemap_scan_backout_range(),
> that page_region is always allocated for p->vec_buf_index.
>
> Fix it by explicitly checking cur_buf for NULL before dereferencing.
>
> Other sites that might run into same deref-issue are already (directly
> or transitively) protected by checking p->vec_buf.
>
> Note:
> From PAGEMAP_SCAN man page, it seems vec_len = 0 is valid when no output
> is requested and it's only the side effects caller is interested in,
> hence it passes check in pagemap_scan_get_args().
>
> This issue was found by syzkaller.
>
> Fixes: 52526ca7fdb9 ("fs/proc/task_mmu: implement IOCTL to get and optionally clear info about PTEs")
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Cc: Jinjiang Tu <tujinjiang@huawei.com>
> Cc: Suren Baghdasaryan <surenb@google.com>
> Cc: Penglei Jiang <superman.xpt@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: Andrei Vagin <avagin@gmail.com>
> Cc: "Michał Mirosław" <mirq-linux@rere.qmqm.pl>
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
> linux-kernel@vger.kernel.org
> linux-fsdevel@vger.kernel.org
> Cc: stable@vger.kernel.org
> Signed-off-by: Jakub Acs <acsjakub@amazon.de>
>
> ---
>  fs/proc/task_mmu.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 29cca0e6d0ff..8c10a8135e74 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -2417,6 +2417,9 @@ static void pagemap_scan_backout_range(struct pagemap_scan_private *p,
>  {
>         struct page_region *cur_buf = &p->vec_buf[p->vec_buf_index];
>
> +       if (!cur_buf)

I think it is better to check !p->vec_buf. I know that vec_buf_index is
always 0 in this case, so there is no functional difference, but the
!p->vec_buf is more readable/obvious.

Thanks,
Andrei

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fs/proc/task_mmu: check cur_buf for NULL
  2025-09-19 16:22 ` Andrei Vagin
@ 2025-09-22  7:24   ` Jakub Acs
  2025-09-22  7:57     ` David Hildenbrand
  0 siblings, 1 reply; 5+ messages in thread
From: Jakub Acs @ 2025-09-22  7:24 UTC (permalink / raw)
  To: Andrei Vagin
  Cc: linux-fsdevel, Andrew Morton, David Hildenbrand, Vlastimil Babka,
	Lorenzo Stoakes, Jinjiang Tu, Suren Baghdasaryan, Penglei Jiang,
	Mark Brown, Baolin Wang, Ryan Roberts, Michał Mirosław,
	Stephen Rothwell, Muhammad Usama Anjum, stable

On Fri, Sep 19, 2025 at 09:22:14AM -0700, Andrei Vagin wrote:
> On Fri, Sep 19, 2025 at 7:21 AM Jakub Acs <acsjakub@amazon.de> wrote:
> >
> > When PAGEMAP_SCAN ioctl invoked with vec_len = 0 reaches
> > pagemap_scan_backout_range(), kernel panics with null-ptr-deref:
> >
> > [   44.936808] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN NOPTI
> > [   44.937797] KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> > [   44.938391] CPU: 1 UID: 0 PID: 2480 Comm: reproducer Not tainted 6.17.0-rc6 #22 PREEMPT(none)
> > [   44.939062] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
> > [   44.939935] RIP: 0010:pagemap_scan_thp_entry.isra.0+0x741/0xa80
> >
> > <snip registers, unreliable trace>
> >
> > [   44.946828] Call Trace:
> > [   44.947030]  <TASK>
> > [   44.949219]  pagemap_scan_pmd_entry+0xec/0xfa0
> > [   44.952593]  walk_pmd_range.isra.0+0x302/0x910
> > [   44.954069]  walk_pud_range.isra.0+0x419/0x790
> > [   44.954427]  walk_p4d_range+0x41e/0x620
> > [   44.954743]  walk_pgd_range+0x31e/0x630
> > [   44.955057]  __walk_page_range+0x160/0x670
> > [   44.956883]  walk_page_range_mm+0x408/0x980
> > [   44.958677]  walk_page_range+0x66/0x90
> > [   44.958984]  do_pagemap_scan+0x28d/0x9c0
> > [   44.961833]  do_pagemap_cmd+0x59/0x80
> > [   44.962484]  __x64_sys_ioctl+0x18d/0x210
> > [   44.962804]  do_syscall_64+0x5b/0x290
> > [   44.963111]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> >
> > vec_len = 0 in pagemap_scan_init_bounce_buffer() means no buffers are
> > allocated and p->vec_buf remains set to NULL.
> >
> > This breaks an assumption made later in pagemap_scan_backout_range(),
> > that page_region is always allocated for p->vec_buf_index.
> >
> > Fix it by explicitly checking cur_buf for NULL before dereferencing.
> >
> > Other sites that might run into same deref-issue are already (directly
> > or transitively) protected by checking p->vec_buf.
> >
> > Note:
> > From PAGEMAP_SCAN man page, it seems vec_len = 0 is valid when no output
> > is requested and it's only the side effects caller is interested in,
> > hence it passes check in pagemap_scan_get_args().
> >
> > This issue was found by syzkaller.
> >
> > Fixes: 52526ca7fdb9 ("fs/proc/task_mmu: implement IOCTL to get and optionally clear info about PTEs")
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Vlastimil Babka <vbabka@suse.cz>
> > Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > Cc: Jinjiang Tu <tujinjiang@huawei.com>
> > Cc: Suren Baghdasaryan <surenb@google.com>
> > Cc: Penglei Jiang <superman.xpt@gmail.com>
> > Cc: Mark Brown <broonie@kernel.org>
> > Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
> > Cc: Ryan Roberts <ryan.roberts@arm.com>
> > Cc: Andrei Vagin <avagin@gmail.com>
> > Cc: "Michał Mirosław" <mirq-linux@rere.qmqm.pl>
> > Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> > Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
> > linux-kernel@vger.kernel.org
> > linux-fsdevel@vger.kernel.org
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jakub Acs <acsjakub@amazon.de>
> >
> > ---
> >  fs/proc/task_mmu.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> > index 29cca0e6d0ff..8c10a8135e74 100644
> > --- a/fs/proc/task_mmu.c
> > +++ b/fs/proc/task_mmu.c
> > @@ -2417,6 +2417,9 @@ static void pagemap_scan_backout_range(struct pagemap_scan_private *p,
> >  {
> >         struct page_region *cur_buf = &p->vec_buf[p->vec_buf_index];
> >
> > +       if (!cur_buf)
> 
> I think it is better to check !p->vec_buf. I know that vec_buf_index is
> always 0 in this case, so there is no functional difference, but the
> !p->vec_buf is more readable/obvious.
> 
> Thanks,
> Andrei

I chose (!cur_buf) because it is more 'paranoid' than !p->vec_buf,
but happy to change that in v2. However, I noticed that the patch was
already merged to mm-hotfixes-unstable in [1]. Should I still send the
v2 with adjustment?

[1]: https://lore.kernel.org/all/20250919201404.1ACB9C4CEF0@smtp.kernel.org/



Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christian Schlaeger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fs/proc/task_mmu: check cur_buf for NULL
  2025-09-22  7:24   ` Jakub Acs
@ 2025-09-22  7:57     ` David Hildenbrand
  2025-09-22  8:25       ` Jakub Acs
  0 siblings, 1 reply; 5+ messages in thread
From: David Hildenbrand @ 2025-09-22  7:57 UTC (permalink / raw)
  To: Jakub Acs, Andrei Vagin
  Cc: linux-fsdevel, Andrew Morton, Vlastimil Babka, Lorenzo Stoakes,
	Jinjiang Tu, Suren Baghdasaryan, Penglei Jiang, Mark Brown,
	Baolin Wang, Ryan Roberts, Michał Mirosław,
	Stephen Rothwell, Muhammad Usama Anjum, stable

On 22.09.25 09:24, Jakub Acs wrote:
> On Fri, Sep 19, 2025 at 09:22:14AM -0700, Andrei Vagin wrote:
>> On Fri, Sep 19, 2025 at 7:21 AM Jakub Acs <acsjakub@amazon.de> wrote:
>>>
>>> When PAGEMAP_SCAN ioctl invoked with vec_len = 0 reaches
>>> pagemap_scan_backout_range(), kernel panics with null-ptr-deref:
>>>
>>> [   44.936808] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN NOPTI
>>> [   44.937797] KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
>>> [   44.938391] CPU: 1 UID: 0 PID: 2480 Comm: reproducer Not tainted 6.17.0-rc6 #22 PREEMPT(none)
>>> [   44.939062] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
>>> [   44.939935] RIP: 0010:pagemap_scan_thp_entry.isra.0+0x741/0xa80
>>>
>>> <snip registers, unreliable trace>
>>>
>>> [   44.946828] Call Trace:
>>> [   44.947030]  <TASK>
>>> [   44.949219]  pagemap_scan_pmd_entry+0xec/0xfa0
>>> [   44.952593]  walk_pmd_range.isra.0+0x302/0x910
>>> [   44.954069]  walk_pud_range.isra.0+0x419/0x790
>>> [   44.954427]  walk_p4d_range+0x41e/0x620
>>> [   44.954743]  walk_pgd_range+0x31e/0x630
>>> [   44.955057]  __walk_page_range+0x160/0x670
>>> [   44.956883]  walk_page_range_mm+0x408/0x980
>>> [   44.958677]  walk_page_range+0x66/0x90
>>> [   44.958984]  do_pagemap_scan+0x28d/0x9c0
>>> [   44.961833]  do_pagemap_cmd+0x59/0x80
>>> [   44.962484]  __x64_sys_ioctl+0x18d/0x210
>>> [   44.962804]  do_syscall_64+0x5b/0x290
>>> [   44.963111]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>>
>>> vec_len = 0 in pagemap_scan_init_bounce_buffer() means no buffers are
>>> allocated and p->vec_buf remains set to NULL.
>>>
>>> This breaks an assumption made later in pagemap_scan_backout_range(),
>>> that page_region is always allocated for p->vec_buf_index.
>>>
>>> Fix it by explicitly checking cur_buf for NULL before dereferencing.
>>>
>>> Other sites that might run into same deref-issue are already (directly
>>> or transitively) protected by checking p->vec_buf.
>>>
>>> Note:
>>>  From PAGEMAP_SCAN man page, it seems vec_len = 0 is valid when no output
>>> is requested and it's only the side effects caller is interested in,
>>> hence it passes check in pagemap_scan_get_args().
>>>
>>> This issue was found by syzkaller.
>>>
>>> Fixes: 52526ca7fdb9 ("fs/proc/task_mmu: implement IOCTL to get and optionally clear info about PTEs")
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Cc: Vlastimil Babka <vbabka@suse.cz>
>>> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>> Cc: Jinjiang Tu <tujinjiang@huawei.com>
>>> Cc: Suren Baghdasaryan <surenb@google.com>
>>> Cc: Penglei Jiang <superman.xpt@gmail.com>
>>> Cc: Mark Brown <broonie@kernel.org>
>>> Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
>>> Cc: Ryan Roberts <ryan.roberts@arm.com>
>>> Cc: Andrei Vagin <avagin@gmail.com>
>>> Cc: "Michał Mirosław" <mirq-linux@rere.qmqm.pl>
>>> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
>>> Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
>>> linux-kernel@vger.kernel.org
>>> linux-fsdevel@vger.kernel.org
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Jakub Acs <acsjakub@amazon.de>
>>>
>>> ---
>>>   fs/proc/task_mmu.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
>>> index 29cca0e6d0ff..8c10a8135e74 100644
>>> --- a/fs/proc/task_mmu.c
>>> +++ b/fs/proc/task_mmu.c
>>> @@ -2417,6 +2417,9 @@ static void pagemap_scan_backout_range(struct pagemap_scan_private *p,
>>>   {
>>>          struct page_region *cur_buf = &p->vec_buf[p->vec_buf_index];
>>>
>>> +       if (!cur_buf)
>>
>> I think it is better to check !p->vec_buf. I know that vec_buf_index is
>> always 0 in this case, so there is no functional difference, but the
>> !p->vec_buf is more readable/obvious.

Yes, please check p->vec_buf like we do in pagemap_scan_output().

> 
> I chose (!cur_buf) because it is more 'paranoid' than !p->vec_buf,
> but happy to change that in v2. However, I noticed that the patch was
> already merged to mm-hotfixes-unstable in [1]. Should I still send the
> v2 with adjustment?

Feel free to send a quick fixup inline or resend the v2.

As long as it's not in -stable we can change it as we please.

-- 
Cheers

David / dhildenb


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fs/proc/task_mmu: check cur_buf for NULL
  2025-09-22  7:57     ` David Hildenbrand
@ 2025-09-22  8:25       ` Jakub Acs
  0 siblings, 0 replies; 5+ messages in thread
From: Jakub Acs @ 2025-09-22  8:25 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Andrei Vagin, linux-fsdevel, Andrew Morton, Vlastimil Babka,
	Lorenzo Stoakes, Jinjiang Tu, Suren Baghdasaryan, Penglei Jiang,
	Mark Brown, Baolin Wang, Ryan Roberts, Michał Mirosław,
	Stephen Rothwell, Muhammad Usama Anjum, stable

On Mon, Sep 22, 2025 at 09:57:48AM +0200, David Hildenbrand wrote:
> On 22.09.25 09:24, Jakub Acs wrote:
> >On Fri, Sep 19, 2025 at 09:22:14AM -0700, Andrei Vagin wrote:
> >>On Fri, Sep 19, 2025 at 7:21 AM Jakub Acs <acsjakub@amazon.de> wrote:
> >>>
> >>>When PAGEMAP_SCAN ioctl invoked with vec_len = 0 reaches
> >>>pagemap_scan_backout_range(), kernel panics with null-ptr-deref:
> >>>
> >>>[   44.936808] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN NOPTI
> >>>[   44.937797] KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> >>>[   44.938391] CPU: 1 UID: 0 PID: 2480 Comm: reproducer Not tainted 6.17.0-rc6 #22 PREEMPT(none)
> >>>[   44.939062] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
> >>>[   44.939935] RIP: 0010:pagemap_scan_thp_entry.isra.0+0x741/0xa80
> >>>
> >>><snip registers, unreliable trace>
> >>>
> >>>[   44.946828] Call Trace:
> >>>[   44.947030]  <TASK>
> >>>[   44.949219]  pagemap_scan_pmd_entry+0xec/0xfa0
> >>>[   44.952593]  walk_pmd_range.isra.0+0x302/0x910
> >>>[   44.954069]  walk_pud_range.isra.0+0x419/0x790
> >>>[   44.954427]  walk_p4d_range+0x41e/0x620
> >>>[   44.954743]  walk_pgd_range+0x31e/0x630
> >>>[   44.955057]  __walk_page_range+0x160/0x670
> >>>[   44.956883]  walk_page_range_mm+0x408/0x980
> >>>[   44.958677]  walk_page_range+0x66/0x90
> >>>[   44.958984]  do_pagemap_scan+0x28d/0x9c0
> >>>[   44.961833]  do_pagemap_cmd+0x59/0x80
> >>>[   44.962484]  __x64_sys_ioctl+0x18d/0x210
> >>>[   44.962804]  do_syscall_64+0x5b/0x290
> >>>[   44.963111]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> >>>
> >>>vec_len = 0 in pagemap_scan_init_bounce_buffer() means no buffers are
> >>>allocated and p->vec_buf remains set to NULL.
> >>>
> >>>This breaks an assumption made later in pagemap_scan_backout_range(),
> >>>that page_region is always allocated for p->vec_buf_index.
> >>>
> >>>Fix it by explicitly checking cur_buf for NULL before dereferencing.
> >>>
> >>>Other sites that might run into same deref-issue are already (directly
> >>>or transitively) protected by checking p->vec_buf.
> >>>
> >>>Note:
> >>> From PAGEMAP_SCAN man page, it seems vec_len = 0 is valid when no output
> >>>is requested and it's only the side effects caller is interested in,
> >>>hence it passes check in pagemap_scan_get_args().
> >>>
> >>>This issue was found by syzkaller.
> >>>
> >>>Fixes: 52526ca7fdb9 ("fs/proc/task_mmu: implement IOCTL to get and optionally clear info about PTEs")
> >>>Cc: Andrew Morton <akpm@linux-foundation.org>
> >>>Cc: David Hildenbrand <david@redhat.com>
> >>>Cc: Vlastimil Babka <vbabka@suse.cz>
> >>>Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> >>>Cc: Jinjiang Tu <tujinjiang@huawei.com>
> >>>Cc: Suren Baghdasaryan <surenb@google.com>
> >>>Cc: Penglei Jiang <superman.xpt@gmail.com>
> >>>Cc: Mark Brown <broonie@kernel.org>
> >>>Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
> >>>Cc: Ryan Roberts <ryan.roberts@arm.com>
> >>>Cc: Andrei Vagin <avagin@gmail.com>
> >>>Cc: "Michał Mirosław" <mirq-linux@rere.qmqm.pl>
> >>>Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> >>>Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
> >>>linux-kernel@vger.kernel.org
> >>>linux-fsdevel@vger.kernel.org
> >>>Cc: stable@vger.kernel.org
> >>>Signed-off-by: Jakub Acs <acsjakub@amazon.de>
> >>>
> >>>---
> >>>  fs/proc/task_mmu.c | 3 +++
> >>>  1 file changed, 3 insertions(+)
> >>>
> >>>diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> >>>index 29cca0e6d0ff..8c10a8135e74 100644
> >>>--- a/fs/proc/task_mmu.c
> >>>+++ b/fs/proc/task_mmu.c
> >>>@@ -2417,6 +2417,9 @@ static void pagemap_scan_backout_range(struct pagemap_scan_private *p,
> >>>  {
> >>>         struct page_region *cur_buf = &p->vec_buf[p->vec_buf_index];
> >>>
> >>>+       if (!cur_buf)
> >>
> >>I think it is better to check !p->vec_buf. I know that vec_buf_index is
> >>always 0 in this case, so there is no functional difference, but the
> >>!p->vec_buf is more readable/obvious.
> 
> Yes, please check p->vec_buf like we do in pagemap_scan_output().
> 
> >
> >I chose (!cur_buf) because it is more 'paranoid' than !p->vec_buf,
> >but happy to change that in v2. However, I noticed that the patch was
> >already merged to mm-hotfixes-unstable in [1]. Should I still send the
> >v2 with adjustment?
> 
> Feel free to send a quick fixup inline or resend the v2.
> 
> As long as it's not in -stable we can change it as we please.

Great,

I screwed up the commit title in v2, so v3 is here:
https://lore.kernel.org/all/20250922082206.6889-1-acsjakub@amazon.de/

Thanks to all,
Jakub



Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christian Schlaeger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-09-22  8:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-19 14:21 [PATCH] fs/proc/task_mmu: check cur_buf for NULL Jakub Acs
2025-09-19 16:22 ` Andrei Vagin
2025-09-22  7:24   ` Jakub Acs
2025-09-22  7:57     ` David Hildenbrand
2025-09-22  8:25       ` Jakub Acs

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox