Re: [PATCH] arm64: hugetlb: avoid potential NULL dereference

[Mark Rutland <mark.rutland@arm.com>]

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