* [PATCH] arm64: hugetlb: avoid potential NULL dereference
@ 2020-05-05 12:59 Mark Rutland
[not found] ` <CANW9uyt4LorH7cZ+qj51T+yiHwLB+V8d=GjR=yae4APUwyQo+w@mail.gmail.com>
2020-05-06 10:18 ` Catalin Marinas
0 siblings, 2 replies; 4+ messages in thread
From: Mark Rutland @ 2020-05-05 12:59 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Mark Rutland, Catalin Marinas, Will Deacon
The static analyzer in GCC 10 spotted that in huge_pte_alloc() we may
pass a NULL pmdp into pte_alloc_map() when pmd_alloc() returns NULL:
| CC arch/arm64/mm/pageattr.o
| CC arch/arm64/mm/hugetlbpage.o
| from arch/arm64/mm/hugetlbpage.c:10:
| arch/arm64/mm/hugetlbpage.c: In function ‘huge_pte_alloc’:
| ./arch/arm64/include/asm/pgtable-types.h:28:24: warning: dereference of NULL ‘pmdp’ [CWE-690] [-Wanalyzer-null-dereference]
| ./arch/arm64/include/asm/pgtable.h:436:26: note: in expansion of macro ‘pmd_val’
| arch/arm64/mm/hugetlbpage.c:242:10: note: in expansion of macro ‘pte_alloc_map’
| |arch/arm64/mm/hugetlbpage.c:232:10:
| |./arch/arm64/include/asm/pgtable-types.h:28:24:
| ./arch/arm64/include/asm/pgtable.h:436:26: note: in expansion of macro ‘pmd_val’
| arch/arm64/mm/hugetlbpage.c:242:10: note: in expansion of macro ‘pte_alloc_map’
This can only occur when the kernel cannot allocate a page, and so is
unlikely to happen in practice before other systems start failing.
We can avoid this by bailing out if pmd_alloc() fails, as we do earlier
in the function if pud_alloc() fails.
Fixes: 66b3923a1a0f77a5 ("arm64: hugetlb: add support for PTE contiguous bit)"
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reported-by: Kyrill Tkachov <kyrylo.tkachov@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/mm/hugetlbpage.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index bbeb6a5a6ba6..0be3355e3499 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -230,6 +230,8 @@ pte_t *huge_pte_alloc(struct mm_struct *mm,
ptep = (pte_t *)pudp;
} else if (sz == (CONT_PTE_SIZE)) {
pmdp = pmd_alloc(mm, pudp, addr);
+ if (!pmdp)
+ return NULL;
WARN_ON(addr & (sz - 1));
/*
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] arm64: hugetlb: avoid potential NULL dereference
[not found] ` <CANW9uyt4LorH7cZ+qj51T+yiHwLB+V8d=GjR=yae4APUwyQo+w@mail.gmail.com>
@ 2020-05-05 14:31 ` Mark Rutland
2020-05-06 2:00 ` Itaru Kitayama
0 siblings, 1 reply; 4+ messages in thread
From: Mark Rutland @ 2020-05-05 14:31 UTC (permalink / raw)
To: Itaru Kitayama; +Cc: Catalin Marinas, Will Deacon, linux-arm-kernel
On Tue, May 05, 2020 at 10:12:02PM +0900, Itaru Kitayama wrote:
> The overhead is just negligible in the paths?
Sorry, I'm not sure I understand your question. Are you asking if this
is likely to affect performance?
I don't expect there to be any measureable overhead here. Practically
speaking the difference is a CBZ + RET, and other factors will dominate
here.
Regardless, this is trivial error handling that was missing from the
original patch.
Thanks,
Mark.
> On Tue, May 5, 2020 at 21:59 Mark Rutland <mark.rutland@arm.com> wrote:
>
> > The static analyzer in GCC 10 spotted that in huge_pte_alloc() we may
> > pass a NULL pmdp into pte_alloc_map() when pmd_alloc() returns NULL:
> >
> > | CC arch/arm64/mm/pageattr.o
> > | CC arch/arm64/mm/hugetlbpage.o
> > | from arch/arm64/mm/hugetlbpage.c:10:
> > | arch/arm64/mm/hugetlbpage.c: In function ‘huge_pte_alloc’:
> > | ./arch/arm64/include/asm/pgtable-types.h:28:24: warning: dereference of
> > NULL ‘pmdp’ [CWE-690] [-Wanalyzer-null-dereference]
> > | ./arch/arm64/include/asm/pgtable.h:436:26: note: in expansion of macro
> > ‘pmd_val’
> > | arch/arm64/mm/hugetlbpage.c:242:10: note: in expansion of macro
> > ‘pte_alloc_map’
> > | |arch/arm64/mm/hugetlbpage.c:232:10:
> > | |./arch/arm64/include/asm/pgtable-types.h:28:24:
> > | ./arch/arm64/include/asm/pgtable.h:436:26: note: in expansion of macro
> > ‘pmd_val’
> > | arch/arm64/mm/hugetlbpage.c:242:10: note: in expansion of macro
> > ‘pte_alloc_map’
> >
> > This can only occur when the kernel cannot allocate a page, and so is
> > unlikely to happen in practice before other systems start failing.
> >
> > We can avoid this by bailing out if pmd_alloc() fails, as we do earlier
> > in the function if pud_alloc() fails.
> >
> > Fixes: 66b3923a1a0f77a5 ("arm64: hugetlb: add support for PTE contiguous
> > bit)"
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Reported-by: Kyrill Tkachov <kyrylo.tkachov@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > ---
> > arch/arm64/mm/hugetlbpage.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> > index bbeb6a5a6ba6..0be3355e3499 100644
> > --- a/arch/arm64/mm/hugetlbpage.c
> > +++ b/arch/arm64/mm/hugetlbpage.c
> > @@ -230,6 +230,8 @@ pte_t *huge_pte_alloc(struct mm_struct *mm,
> > ptep = (pte_t *)pudp;
> > } else if (sz == (CONT_PTE_SIZE)) {
> > pmdp = pmd_alloc(mm, pudp, addr);
> > + if (!pmdp)
> > + return NULL;
> >
> > WARN_ON(addr & (sz - 1));
> > /*
> > --
> > 2.11.0
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] arm64: hugetlb: avoid potential NULL dereference
2020-05-05 14:31 ` Mark Rutland
@ 2020-05-06 2:00 ` Itaru Kitayama
0 siblings, 0 replies; 4+ messages in thread
From: Itaru Kitayama @ 2020-05-06 2:00 UTC (permalink / raw)
To: Mark Rutland; +Cc: Catalin Marinas, Will Deacon, Linux ARM
Hi Mark,
I looked in the mm and arch/arm64 directories and all the calls to
pmd_alloc() are tested
whether it returns a NULL or not, so your patch just fixes the hugetlb
code to follow the standard.
On Tue, May 5, 2020 at 11:31 PM Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Tue, May 05, 2020 at 10:12:02PM +0900, Itaru Kitayama wrote:
> > The overhead is just negligible in the paths?
>
> Sorry, I'm not sure I understand your question. Are you asking if this
> is likely to affect performance?
>
> I don't expect there to be any measureable overhead here. Practically
> speaking the difference is a CBZ + RET, and other factors will dominate
> here.
>
> Regardless, this is trivial error handling that was missing from the
> original patch.
>
> Thanks,
> Mark.
>
> > On Tue, May 5, 2020 at 21:59 Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > > The static analyzer in GCC 10 spotted that in huge_pte_alloc() we may
> > > pass a NULL pmdp into pte_alloc_map() when pmd_alloc() returns NULL:
> > >
> > > | CC arch/arm64/mm/pageattr.o
> > > | CC arch/arm64/mm/hugetlbpage.o
> > > | from arch/arm64/mm/hugetlbpage.c:10:
> > > | arch/arm64/mm/hugetlbpage.c: In function ‘huge_pte_alloc’:
> > > | ./arch/arm64/include/asm/pgtable-types.h:28:24: warning: dereference of
> > > NULL ‘pmdp’ [CWE-690] [-Wanalyzer-null-dereference]
> > > | ./arch/arm64/include/asm/pgtable.h:436:26: note: in expansion of macro
> > > ‘pmd_val’
> > > | arch/arm64/mm/hugetlbpage.c:242:10: note: in expansion of macro
> > > ‘pte_alloc_map’
> > > | |arch/arm64/mm/hugetlbpage.c:232:10:
> > > | |./arch/arm64/include/asm/pgtable-types.h:28:24:
> > > | ./arch/arm64/include/asm/pgtable.h:436:26: note: in expansion of macro
> > > ‘pmd_val’
> > > | arch/arm64/mm/hugetlbpage.c:242:10: note: in expansion of macro
> > > ‘pte_alloc_map’
> > >
> > > This can only occur when the kernel cannot allocate a page, and so is
> > > unlikely to happen in practice before other systems start failing.
> > >
> > > We can avoid this by bailing out if pmd_alloc() fails, as we do earlier
> > > in the function if pud_alloc() fails.
> > >
> > > Fixes: 66b3923a1a0f77a5 ("arm64: hugetlb: add support for PTE contiguous
> > > bit)"
> > > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > > Reported-by: Kyrill Tkachov <kyrylo.tkachov@arm.com>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > Cc: Will Deacon <will@kernel.org>
> > > ---
> > > arch/arm64/mm/hugetlbpage.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> > > index bbeb6a5a6ba6..0be3355e3499 100644
> > > --- a/arch/arm64/mm/hugetlbpage.c
> > > +++ b/arch/arm64/mm/hugetlbpage.c
> > > @@ -230,6 +230,8 @@ pte_t *huge_pte_alloc(struct mm_struct *mm,
> > > ptep = (pte_t *)pudp;
> > > } else if (sz == (CONT_PTE_SIZE)) {
> > > pmdp = pmd_alloc(mm, pudp, addr);
> > > + if (!pmdp)
> > > + return NULL;
> > >
> > > WARN_ON(addr & (sz - 1));
> > > /*
> > > --
> > > 2.11.0
> > >
> > >
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > linux-arm-kernel@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] arm64: hugetlb: avoid potential NULL dereference
2020-05-05 12:59 [PATCH] arm64: hugetlb: avoid potential NULL dereference Mark Rutland
[not found] ` <CANW9uyt4LorH7cZ+qj51T+yiHwLB+V8d=GjR=yae4APUwyQo+w@mail.gmail.com>
@ 2020-05-06 10:18 ` Catalin Marinas
1 sibling, 0 replies; 4+ messages in thread
From: Catalin Marinas @ 2020-05-06 10:18 UTC (permalink / raw)
To: Mark Rutland; +Cc: Will Deacon, linux-arm-kernel
On Tue, May 05, 2020 at 01:59:30PM +0100, Mark Rutland wrote:
> The static analyzer in GCC 10 spotted that in huge_pte_alloc() we may
> pass a NULL pmdp into pte_alloc_map() when pmd_alloc() returns NULL:
>
> | CC arch/arm64/mm/pageattr.o
> | CC arch/arm64/mm/hugetlbpage.o
> | from arch/arm64/mm/hugetlbpage.c:10:
> | arch/arm64/mm/hugetlbpage.c: In function ‘huge_pte_alloc’:
> | ./arch/arm64/include/asm/pgtable-types.h:28:24: warning: dereference of NULL ‘pmdp’ [CWE-690] [-Wanalyzer-null-dereference]
> | ./arch/arm64/include/asm/pgtable.h:436:26: note: in expansion of macro ‘pmd_val’
> | arch/arm64/mm/hugetlbpage.c:242:10: note: in expansion of macro ‘pte_alloc_map’
> | |arch/arm64/mm/hugetlbpage.c:232:10:
> | |./arch/arm64/include/asm/pgtable-types.h:28:24:
> | ./arch/arm64/include/asm/pgtable.h:436:26: note: in expansion of macro ‘pmd_val’
> | arch/arm64/mm/hugetlbpage.c:242:10: note: in expansion of macro ‘pte_alloc_map’
>
> This can only occur when the kernel cannot allocate a page, and so is
> unlikely to happen in practice before other systems start failing.
>
> We can avoid this by bailing out if pmd_alloc() fails, as we do earlier
> in the function if pud_alloc() fails.
>
> Fixes: 66b3923a1a0f77a5 ("arm64: hugetlb: add support for PTE contiguous bit)"
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Reported-by: Kyrill Tkachov <kyrylo.tkachov@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
Queued for 5.7-rc5. Thanks.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-06 10:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-05 12:59 [PATCH] arm64: hugetlb: avoid potential NULL dereference Mark Rutland
[not found] ` <CANW9uyt4LorH7cZ+qj51T+yiHwLB+V8d=GjR=yae4APUwyQo+w@mail.gmail.com>
2020-05-05 14:31 ` Mark Rutland
2020-05-06 2:00 ` Itaru Kitayama
2020-05-06 10:18 ` Catalin Marinas
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.