Re: [PATCH v2 2/2] KVM: selftests: Verify failed IOAPIC delivery preserves state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Uninitialized `struct kvm_irqchip` passed to `KVM_GET_IRQCHIP` ioctl leaks stack memory to the kernel, triggering memory sanitizer errors.
--

--- Patch [2]: [PATCH v2 2/2] KVM: selftests: Verify failed IOAPIC delivery preserves state ---
commit bdb3cd0c37699df03cd34fd3d221e67589076c39
Author: Hao Zhang <zhanghao1@kylinos.cn>

KVM: selftests: Verify failed IOAPIC delivery preserves state

This commit adds regression coverage for I/O APIC interrupt delivery when no
local APIC can accept the interrupt. It verifies that failed delivery behaves
correctly for both level-triggered and edge-triggered interrupts.

> diff --git a/tools/testing/selftests/kvm/x86/ioapic_state_test.c b/tools/testing/selftests/kvm/x86/ioapic_state_test.c
> new file mode 100644
> index 0000000000000..3613a67f4b217
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86/ioapic_state_test.c
[ ... ]
> +static void set_undeliverable_ioapic_entry(struct kvm_vm *vm,
> +					   bool level_triggered)
> +{
> +	struct kvm_irqchip irqchip;
> +
> +	get_ioapic(vm, &irqchip);

[Severity: Medium]
Will this leak uninitialized stack memory to the kernel?

Since KVM_GET_IRQCHIP is defined as _IOWR, the kernel uses memdup_user() to
copy the entire struct kvm_irqchip from userspace. Because the struct contains
a 512-byte union and is declared uninitialized on the stack, passing it to the
kernel will trigger memory sanitizer or Valgrind errors during testing.

Could this be zero-initialized?

[ ... ]
> +static void test_no_remote_irr_for_undelivered_interrupt(void)
> +{
> +	struct kvm_irq_level irq = {
> +		.irq = TEST_IOAPIC_PIN,
> +		.level = 1,
> +	};
> +	struct kvm_irqchip irqchip;
> +	struct kvm_vm *vm;
> +
> +	vm = vm_create_barebones();
> +	vm_create_irqchip(vm);
> +
> +	set_undeliverable_ioapic_entry(vm, true);
> +
> +	vm_ioctl(vm, KVM_IRQ_LINE_STATUS, &irq);
> +	TEST_ASSERT(irq.status == -1,
> +		    "Expected failed interrupt delivery, got %d", irq.status);
> +
> +	get_ioapic(vm, &irqchip);

[Severity: Medium]
Does this struct also need to be zero-initialized to prevent memory
sanitizer errors when calling the KVM_GET_IRQCHIP ioctl?

[ ... ]
> +static void test_undelivered_edge_interrupt_stays_pending(void)
> +{
> +	struct kvm_irq_level irq = {
> +		.irq = TEST_IOAPIC_PIN,
> +		.level = 1,
> +	};
> +	struct kvm_irqchip irqchip;
> +	struct kvm_vm *vm;
> +
> +	vm = vm_create_barebones();
> +	vm_create_irqchip(vm);
> +
> +	set_undeliverable_ioapic_entry(vm, false);
> +
> +	vm_ioctl(vm, KVM_IRQ_LINE_STATUS, &irq);
> +	TEST_ASSERT(irq.status == -1,
> +		    "Expected failed interrupt delivery, got %d", irq.status);
> +
> +	get_ioapic(vm, &irqchip);

[Severity: Medium]
Does this uninitialized struct also need to be zero-initialized for the
same reason?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/anls-aUppuiFolpS@192.168.1.215?part=2