Re: [PATCH] KVM: x86: Fix null pointer dereference in trace_kvm_inj_exception

[sashiko-bot@kernel.org]

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