Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH] KVM: x86: Fix null pointer dereference in trace_kvm_inj_exception
@ 2026-07-10 13:40 Daniel Paziyski
  2026-07-10 13:53 ` sashiko-bot
  2026-07-10 18:42 ` Sean Christopherson
  0 siblings, 2 replies; 5+ messages in thread
From: Daniel Paziyski @ 2026-07-10 13:40 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86
  Cc: Daniel Paziyski, H. Peter Anvin, kvm, linux-kernel

The trace_kvm_inj_exception tracepoint takes as arguments the exception
vector, whether the exception has an error code (and subsequently, the
error code), and whether it is being reinjected. The error code is
formatted to the trace log as a string using __print_symbolic,
which takes an integer value and an array of structs containing a mask
value and a string. An empty struct is being passed, which makes the
kernel derefence a NULL pointer when the error code is 0. Remove this
stray struct in the invocation of __print_symbolic.

Signed-off-by: Daniel Paziyski <danielpaziyski@gmail.com>
---
 arch/x86/kvm/trace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 0db25bba1..93de876c3 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -490,7 +490,7 @@ TRACE_EVENT(kvm_inj_exception,
 	TP_printk("%s%s%s%s%s",
 		  __print_symbolic(__entry->exception, kvm_trace_sym_exc),
 		  !__entry->has_error ? "" : " (",
-		  !__entry->has_error ? "" : __print_symbolic(__entry->error_code, { }),
+		  !__entry->has_error ? "" : __print_symbolic(__entry->error_code),
 		  !__entry->has_error ? "" : ")",
 		  __entry->reinjected ? " [reinjected]" : "")
 );
-- 
2.55.0


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

* Re: [PATCH] KVM: x86: Fix null pointer dereference in trace_kvm_inj_exception
  2026-07-10 13:40 [PATCH] KVM: x86: Fix null pointer dereference in trace_kvm_inj_exception Daniel Paziyski
@ 2026-07-10 13:53 ` sashiko-bot
  2026-07-10 18:35   ` Sean Christopherson
  2026-07-10 18:42 ` Sean Christopherson
  1 sibling, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-07-10 13:53 UTC (permalink / raw)
  To: Daniel Paziyski; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The proposed fix is based on a false premise. The commit message incorrectly claims the original code causes a kernel NULL pointer dereference, but the original code safely handled the empty struct.
- [Low] Removing the variadic argument from the `__print_symbolic` macro causes a syntax error during macro expansion, which unconditionally breaks the kernel build.
--

--- Patch [1]: [PATCH] KVM: x86: Fix null pointer dereference in trace_kvm_inj_exception ---
commit ed0cb139106d0eebc245569ae242561a63dc2bb7
Author: Daniel Paziyski <danielpaziyski@gmail.com>

KVM: x86: Fix null pointer dereference in trace_kvm_inj_exception

This patch modifies the kvm_inj_exception tracepoint to remove an empty
struct passed to __print_symbolic for formatting the exception error code.

> An empty struct is being passed, which makes the
> kernel derefence a NULL pointer when the error code is 0. Remove this
> stray struct in the invocation of __print_symbolic.

[Severity: Medium]
Is this description of a null pointer dereference accurate?

Looking at how __print_symbolic expands, the empty struct { } safely
initializes the first element's name pointer to NULL. When
trace_print_symbols_seq() is called, it handles this gracefully:

kernel/trace/trace_output.c:trace_print_symbols_seq() {
    ...
    for (i = 0;  symbol_array[i].name; i++) {
    ...
}

Since the name field is NULL for the first element, the loop is bypassed
entirely and the function safely falls back to printing the error code as a
hex string without dereferencing a NULL pointer.

> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -490,7 +490,7 @@ TRACE_EVENT(kvm_inj_exception,
>  	TP_printk("%s%s%s%s%s",
>  		  __print_symbolic(__entry->exception, kvm_trace_sym_exc),
>  		  !__entry->has_error ? "" : " (",
> -		  !__entry->has_error ? "" : __print_symbolic(__entry->error_code, { }),
> +		  !__entry->has_error ? "" : __print_symbolic(__entry->error_code),

[Severity: Low]
Will this change break the kernel build?

The __print_symbolic macro expects a symbol_array as the second argument. By
omitting it, the preprocessor will expand the macro to something similar to:

static const struct trace_print_flags symbols[] = { , { -1, NULL }};

Does this result in a syntax error during macro expansion due to the missing
expression before the comma?

>  		  !__entry->has_error ? "" : ")",
>  		  __entry->reinjected ? " [reinjected]" : "")
>  );

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710134055.16432-1-danielpaziyski@gmail.com?part=1

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

* Re: [PATCH] KVM: x86: Fix null pointer dereference in trace_kvm_inj_exception
  2026-07-10 13:53 ` sashiko-bot
@ 2026-07-10 18:35   ` Sean Christopherson
  0 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2026-07-10 18:35 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Daniel Paziyski, kvm

On Fri, Jul 10, 2026, sashiko-bot@kernel.org wrote:
> > An empty struct is being passed, which makes the
> > kernel derefence a NULL pointer when the error code is 0. Remove this
> > stray struct in the invocation of __print_symbolic.
> 
> [Severity: Medium]
> Is this description of a null pointer dereference accurate?

Yes, it is.  Sashiko apparently applied to an old base (pre-7.1).  As of commit
754e38d2d1ae ("tracing: Use explicit array size instead of sentinel elements in
symbol printing") (spoiler alert), trace_print_symbols_seq() is:

	for (i = 0; i < symbol_array_size; i++) {

		if (val != symbol_array[i].mask)
			continue;

		trace_seq_puts(p, symbol_array[i].name);
		break;
	}

> Looking at how __print_symbolic expands, the empty struct { } safely
> initializes the first element's name pointer to NULL. When
> trace_print_symbols_seq() is called, it handles this gracefully:
> 
> kernel/trace/trace_output.c:trace_print_symbols_seq() {
>     ...
>     for (i = 0;  symbol_array[i].name; i++) {
>     ...
> }
> 
> Since the name field is NULL for the first element, the loop is bypassed
> entirely and the function safely falls back to printing the error code as a
> hex string without dereferencing a NULL pointer.
> 
> > diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> > --- a/arch/x86/kvm/trace.h
> > +++ b/arch/x86/kvm/trace.h
> > @@ -490,7 +490,7 @@ TRACE_EVENT(kvm_inj_exception,
> >  	TP_printk("%s%s%s%s%s",
> >  		  __print_symbolic(__entry->exception, kvm_trace_sym_exc),
> >  		  !__entry->has_error ? "" : " (",
> > -		  !__entry->has_error ? "" : __print_symbolic(__entry->error_code, { }),
> > +		  !__entry->has_error ? "" : __print_symbolic(__entry->error_code),
> 
> [Severity: Low]
> Will this change break the kernel build?

No, beacuse again, Sashiko applied to an old base for some reason.

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

* Re: [PATCH] KVM: x86: Fix null pointer dereference in trace_kvm_inj_exception
  2026-07-10 13:40 [PATCH] KVM: x86: Fix null pointer dereference in trace_kvm_inj_exception Daniel Paziyski
  2026-07-10 13:53 ` sashiko-bot
@ 2026-07-10 18:42 ` Sean Christopherson
  2026-07-10 19:34   ` Daniel Paziyski
  1 sibling, 1 reply; 5+ messages in thread
From: Sean Christopherson @ 2026-07-10 18:42 UTC (permalink / raw)
  To: Daniel Paziyski
  Cc: Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, kvm, linux-kernel,
	Thomas Weißschuh

+The Other Thomas

Just as an FYI, commit 754e38d2d1ae ("tracing: Use explicit array size instead of
sentinel elements in symbol printing") broke KVM abuse of __print_symbolic() where
KVM deliberately passed in an "null" array to avoid printing 0x0 when there is no
error code.

I did a half-assed search through the other usage of __print_symbolic() and didn't
see anything, but just in case someone else comes complaining...

On Fri, Jul 10, 2026, Daniel Paziyski wrote:
> The trace_kvm_inj_exception tracepoint takes as arguments the exception
> vector, whether the exception has an error code (and subsequently, the
> error code), and whether it is being reinjected. The error code is
> formatted to the trace log as a string using __print_symbolic,
> which takes an integer value and an array of structs containing a mask
> value and a string. An empty struct is being passed, which makes the
> kernel derefence a NULL pointer when the error code is 0. Remove this
> stray struct in the invocation of __print_symbolic.

Fixes: 754e38d2d1ae ("tracing: Use explicit array size instead of sentinel elements in symbol printing")
Cc: stable@vger.kernel.org

In the future please provide a splat in the changelog, especially in this age of
agentic bug hunters that are prone to making things up.  My initial reaction to
this was "no way".  Thankfully, I tried reproducing the bug before making an ass
out of myself :-)

  BUG: kernel NULL pointer dereference, address: 0000000000000000
  #PF: supervisor read access in kernel mode
  #PF: error_code(0x0000) - not-present page
  PGD 0 P4D 0 
  Oops: Oops: 0000 [#1] SMP
  CPU: 20 UID: 0 PID: 791 Comm: less Not tainted 7.2.0-rc2-3ffd29d795c9-x86_trace_ex_null_ptr_deref-vm #401 PREEMPT 
  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
  RIP: 0010:strlen+0x0/0x20
  Call Trace:
   <TASK>
   trace_seq_puts+0x18/0x80
   trace_print_symbols_seq+0x68/0xa0
   trace_raw_output_kvm_inj_exception+0x64/0xf0 [kvm]
   s_show+0x47/0x110
   seq_read_iter+0x2a5/0x4c0
   seq_read+0xfd/0x130
   vfs_read+0xb6/0x330
   ? vfs_write+0x2f2/0x3f0
   ksys_read+0x61/0xd0
   do_syscall_64+0xb7/0x570
   entry_SYSCALL_64_after_hwframe+0x4b/0x53
  RIP: 0033:0x7ff283714862
   </TASK>

I'll also massage the changelog to explain why KVM is absuing __print_symbolic()
(see commit 21d4c575eb4a ("KVM: x86: Print error code in exception injection
tracepoint iff valid")).

No need for a v2, I'll fixup when applying.

Thanks!

> Signed-off-by: Daniel Paziyski <danielpaziyski@gmail.com>
> ---
>  arch/x86/kvm/trace.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index 0db25bba1..93de876c3 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -490,7 +490,7 @@ TRACE_EVENT(kvm_inj_exception,
>  	TP_printk("%s%s%s%s%s",
>  		  __print_symbolic(__entry->exception, kvm_trace_sym_exc),
>  		  !__entry->has_error ? "" : " (",
> -		  !__entry->has_error ? "" : __print_symbolic(__entry->error_code, { }),
> +		  !__entry->has_error ? "" : __print_symbolic(__entry->error_code),
>  		  !__entry->has_error ? "" : ")",
>  		  __entry->reinjected ? " [reinjected]" : "")
>  );
> -- 
> 2.55.0
> 

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

* Re: [PATCH] KVM: x86: Fix null pointer dereference in trace_kvm_inj_exception
  2026-07-10 18:42 ` Sean Christopherson
@ 2026-07-10 19:34   ` Daniel Paziyski
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Paziyski @ 2026-07-10 19:34 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, kvm, linux-kernel,
	Thomas Weißschuh

Thank you for the response!

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

end of thread, other threads:[~2026-07-10 19:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-10 13:40 [PATCH] KVM: x86: Fix null pointer dereference in trace_kvm_inj_exception Daniel Paziyski
2026-07-10 13:53 ` sashiko-bot
2026-07-10 18:35   ` Sean Christopherson
2026-07-10 18:42 ` Sean Christopherson
2026-07-10 19:34   ` Daniel Paziyski

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