public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/sev: Fix dereference NULL return value
@ 2024-11-19  5:24 Shresth Prasad
  2024-11-19 12:03 ` Markus Elfring
  0 siblings, 1 reply; 7+ messages in thread
From: Shresth Prasad @ 2024-11-19  5:24 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin
  Cc: linux-kernel, Shresth Prasad

Skip to the next CPU if `pte` is assigned NULL by `lookup_address`

This issue was reported by Coverity scan:
https://scan7.scan.coverity.com/#/project-view/52279/11354?selectedIssue=1601527

Signed-off-by: Shresth Prasad <shresthprasad7@gmail.com>
---
 arch/x86/coco/sev/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index c5b0148b8c0a191f5aa38af73a52c77d6bba3e2d..0436366243e19a72bf9521f2e96a3ceec9c1270c 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -1079,6 +1079,8 @@ void snp_kexec_finish(void)
 		data = per_cpu(runtime_data, cpu);
 		ghcb = &data->ghcb_page;
 		pte = lookup_address((unsigned long)ghcb, &level);
+		if (!pte)
+			continue;
 		size = page_level_size(level);
 		set_pte_enc(pte, level, (void *)ghcb);
 		snp_set_memory_private((unsigned long)ghcb, (size / PAGE_SIZE));

---
base-commit: 28955f4fa2823e39f1ecfb3a37a364563527afbc
change-id: 20241119-fix-dereference-null-x86-sev-4b62d89e8b98

Best regards,
-- 
Shresth Prasad <shresthprasad7@gmail.com>


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

* Re: [PATCH] x86/sev: Fix dereference NULL return value
  2024-11-19  5:24 [PATCH] x86/sev: Fix dereference NULL return value Shresth Prasad
@ 2024-11-19 12:03 ` Markus Elfring
  2024-11-19 16:35   ` Shresth Prasad
  0 siblings, 1 reply; 7+ messages in thread
From: Markus Elfring @ 2024-11-19 12:03 UTC (permalink / raw)
  To: Shresth Prasad, x86, Borislav Petkov, Dave Hansen, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner
  Cc: LKML

> Skip to the next CPU if `pte` is assigned NULL by `lookup_address`

                          a null pointer was returned by a lookup_address() call.


* How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.12#n145

* Would a summary phrase like “Prevent null pointer dereference in snp_kexec_finish()”
  be a bit nicer?


Regards,
Markus

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

* Re: [PATCH] x86/sev: Fix dereference NULL return value
  2024-11-19 12:03 ` Markus Elfring
@ 2024-11-19 16:35   ` Shresth Prasad
  2024-11-19 17:33     ` Markus Elfring
  0 siblings, 1 reply; 7+ messages in thread
From: Shresth Prasad @ 2024-11-19 16:35 UTC (permalink / raw)
  To: Markus Elfring
  Cc: x86, Borislav Petkov, Dave Hansen, H. Peter Anvin, Ingo Molnar,
	Thomas Gleixner, LKML

On Tue, Nov 19, 2024 at 5:33 PM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> > Skip to the next CPU if `pte` is assigned NULL by `lookup_address`
>
>                           a null pointer was returned by a lookup_address() call.
>
>
> * How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?

For the Fixes tag, I'm not sure which commit hash I should put. Should I use
the commit where the function was introduced?

>
> * Would a summary phrase like “Prevent null pointer dereference in snp_kexec_finish()”
>   be a bit nicer?

You're right, I'll change the phrasing. Thank you for the feedback.

Best Regards,
Shresth

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

* Re: x86/sev: Fix dereference NULL return value
  2024-11-19 16:35   ` Shresth Prasad
@ 2024-11-19 17:33     ` Markus Elfring
  2024-11-19 18:16       ` Shresth Prasad
  0 siblings, 1 reply; 7+ messages in thread
From: Markus Elfring @ 2024-11-19 17:33 UTC (permalink / raw)
  To: Shresth Prasad, x86
  Cc: Borislav Petkov, Dave Hansen, H. Peter Anvin, Ingo Molnar,
	Thomas Gleixner, LKML

> For the Fixes tag, I'm not sure which commit hash I should put. Should I use
> the commit where the function was introduced?

How far can the overlooked function return value be traced back?

Regards,
Markus

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

* Re: x86/sev: Fix dereference NULL return value
  2024-11-19 17:33     ` Markus Elfring
@ 2024-11-19 18:16       ` Shresth Prasad
  2024-11-19 18:56         ` Markus Elfring
  0 siblings, 1 reply; 7+ messages in thread
From: Shresth Prasad @ 2024-11-19 18:16 UTC (permalink / raw)
  To: Markus Elfring
  Cc: x86, Borislav Petkov, Dave Hansen, H. Peter Anvin, Ingo Molnar,
	Thomas Gleixner, LKML

On Tue, Nov 19, 2024 at 11:03 PM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> > For the Fixes tag, I'm not sure which commit hash I should put. Should I use
> > the commit where the function was introduced?
>
> How far can the overlooked function return value be traced back?

Using git blame I see that snp_kexec_finish() was first introduced in
3074152e56c9b
and has stayed that way since. Should I just use that hash?

Best Regards,
Shresth

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

* Re: x86/sev: Fix dereference NULL return value
  2024-11-19 18:16       ` Shresth Prasad
@ 2024-11-19 18:56         ` Markus Elfring
  2024-11-19 19:54           ` Shresth Prasad
  0 siblings, 1 reply; 7+ messages in thread
From: Markus Elfring @ 2024-11-19 18:56 UTC (permalink / raw)
  To: Shresth Prasad, x86
  Cc: Borislav Petkov, Dave Hansen, H. Peter Anvin, Ingo Molnar,
	Thomas Gleixner, LKML

> Using git blame I see that snp_kexec_finish() was first introduced in
> 3074152e56c9b
> and has stayed that way since. Should I just use that hash?
Probably, yes.
Would you insist to use only 12 characters from such an identifier?

Regards,
Markus

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

* Re: x86/sev: Fix dereference NULL return value
  2024-11-19 18:56         ` Markus Elfring
@ 2024-11-19 19:54           ` Shresth Prasad
  0 siblings, 0 replies; 7+ messages in thread
From: Shresth Prasad @ 2024-11-19 19:54 UTC (permalink / raw)
  To: Markus Elfring
  Cc: x86, Borislav Petkov, Dave Hansen, H. Peter Anvin, Ingo Molnar,
	Thomas Gleixner, LKML

On Wed, Nov 20, 2024 at 12:26 AM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> > Using git blame I see that snp_kexec_finish() was first introduced in
> > 3074152e56c9b
> > and has stayed that way since. Should I just use that hash?
> Probably, yes.
> Would you insist to use only 12 characters from such an identifier?

I'm not sure what you mean but as suggested by the docs I'll use the first 12
characters. Thank you for your help, I'll make these changes and send a v2.

Best Regards,
Shresth

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

end of thread, other threads:[~2024-11-19 19:55 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-19  5:24 [PATCH] x86/sev: Fix dereference NULL return value Shresth Prasad
2024-11-19 12:03 ` Markus Elfring
2024-11-19 16:35   ` Shresth Prasad
2024-11-19 17:33     ` Markus Elfring
2024-11-19 18:16       ` Shresth Prasad
2024-11-19 18:56         ` Markus Elfring
2024-11-19 19:54           ` Shresth Prasad

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