[PATCH V2] x86/mm: Fix NULL pointer dereference in kernel_page_present()

[PATCH V2] x86/mm: Fix NULL pointer dereference in kernel_page_present()

[Tanya Agarwal]

From: Tanya Agarwal <tanyaagarwal25699@gmail.com>

The static code analysis tool "Coverity Scan" pointed the following
details out for further development considerations:
CID 1271215: Dereference null return value (NULL_RETURNS)
dereference: Dereferencing pte, which is known to be NULL.

Conclusion:
Add WARN_ON_ONCE() and NULL check for pte before dereferencing it.

Fixes: 8a235efad548 ("Hibernation: Handle DEBUG_PAGEALLOC on x86")
Signed-off-by: Tanya Agarwal <tanyaagarwal25699@gmail.com>
---
V2: add WARN_ON_ONCE() as suggested by Dave

Coverity Link:
https://scan5.scan.coverity.com/#/project-view/63683/10063?selectedIssue=1271215

 arch/x86/mm/pat/set_memory.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 95bc50a8541c..8f9d418e6a8c 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -2495,6 +2495,9 @@ bool kernel_page_present(struct page *page)
 		return false;
 
 	pte = lookup_address((unsigned long)page_address(page), &level);
+	if (WARN_ON_ONCE(!pte))
+		return false;
+
 	return (pte_val(*pte) & _PAGE_PRESENT);
 }
 
-- 
2.39.5

Re: [PATCH V2] x86/mm: Fix NULL pointer dereference in kernel_page_present()

[Tanya Agarwal]

On Sat, Jan 11, 2025 at 11:28 PM Tanya Agarwal
<tanyaagarwal25699@gmail.com> wrote:
>
> From: Tanya Agarwal <tanyaagarwal25699@gmail.com>
>
> The static code analysis tool "Coverity Scan" pointed the following
> details out for further development considerations:
> CID 1271215: Dereference null return value (NULL_RETURNS)
> dereference: Dereferencing pte, which is known to be NULL.
>
> Conclusion:
> Add WARN_ON_ONCE() and NULL check for pte before dereferencing it.
>
> Fixes: 8a235efad548 ("Hibernation: Handle DEBUG_PAGEALLOC on x86")
> Signed-off-by: Tanya Agarwal <tanyaagarwal25699@gmail.com>
> ---
> V2: add WARN_ON_ONCE() as suggested by Dave
>
> Coverity Link:
> https://scan5.scan.coverity.com/#/project-view/63683/10063?selectedIssue=1271215
>
>  arch/x86/mm/pat/set_memory.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> index 95bc50a8541c..8f9d418e6a8c 100644
> --- a/arch/x86/mm/pat/set_memory.c
> +++ b/arch/x86/mm/pat/set_memory.c
> @@ -2495,6 +2495,9 @@ bool kernel_page_present(struct page *page)
>                 return false;
>
>         pte = lookup_address((unsigned long)page_address(page), &level);
> +       if (WARN_ON_ONCE(!pte))
> +               return false;
> +
>         return (pte_val(*pte) & _PAGE_PRESENT);
>  }
>
> --
> 2.39.5
>

Hi Dave,

I just wanted to check if you've had a chance to review it or need any
more information.

Thanks,
Tanya

Re: [PATCH V2] x86/mm: Fix NULL pointer dereference in kernel_page_present()

[Dave Hansen]

On 1/11/25 09:58, Tanya Agarwal wrote:
> From: Tanya Agarwal <tanyaagarwal25699@gmail.com>
> 
> The static code analysis tool "Coverity Scan" pointed the following
> details out for further development considerations:
> CID 1271215: Dereference null return value (NULL_RETURNS)
> dereference: Dereferencing pte, which is known to be NULL.

I think we need an actual changelog for this, as opposed to just blindly
trusting Coverity. You can say that Coverity helped identify this as an
issue, but we don't need the "CID" or other Coverity gunk in our
changelogs.  We don't care.

Could you please elaborate on where this issue might affect people? It's
also entirely theoretical as far as I can tell. I can't even conjure up
a contrived case where it could be triggered.

We won't do _anything_ with this patch until the merge window closes, so
you've got at least a week to spruce up the changelog.