Re: [PATCH 15/21] KVM: s390: vsie: Add struct vsie_sca with pin and unpin

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Guest-triggerable WARN_ON_ONCE and negative error code truncation in pin_sca().
- [High] Unconditional pinning of 5 pages for ESCA breaks valid guest configurations due to over-pinning.
- [High] The dynamically allocated `struct vsie_sca` leaves its embedded `mutex` and `ref_count` uninitialized.
- [High] Missing NULL pointer check in `free_vsie_sca` can crash the kernel on error paths.
- [Low] Unused static functions trigger compiler warnings and break builds with `-Werror`.
--

--- Patch [15]: [PATCH 15/21] KVM: s390: vsie: Add struct vsie_sca with pin and unpin ---
commit e9a459801d0d13c2945b7f63700e761f8e46bb25
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>

KVM: s390: vsie: Add struct vsie_sca with pin and unpin

Introduce the struct vsie_sca to hold the SSCA and required management
data along with alloc and free methods to use with the struct.

> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
[ ... ]
> @@ -831,6 +854,60 @@ static int pin_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
[ ... ]
> +static int pin_sca(struct kvm *kvm, struct vsie_sca *vsie_sca)
> +{
> +	bool is_esca = test_bit(VSIE_SCA_ESCA, &vsie_sca->flags);
> +	int nr_pages = KVM_S390_MAX_SCA_PAGES;

[Severity: High]
Will this unconditionally pin 5 pages for ESCA blocks, even if the
16,128-byte block only spans 4 pages depending on its alignment?

If a nested guest allocates an ESCA block adjacent to unbacked memory,
could pin_guest_pages() attempt to pin a 5th unbacked page and fail
with -EFAULT, incorrectly rejecting a valid guest configuration?

> +
> +	if (test_bit(VSIE_SCA_PINNED, &vsie_sca->flags))
> +		return 0;
> +
> +	if (!is_esca) {
> +		nr_pages = 1;
> +		if ((vsie_sca->sca_gpa & ~PAGE_MASK) + sizeof(struct bsca_block) > PAGE_SIZE)
> +			nr_pages = 2;
> +	}
> +
> +	vsie_sca->sca_o_nr_pages = pin_guest_pages(kvm, vsie_sca->sca_gpa, nr_pages,
> +						   vsie_sca->sca_o_pages);
> +	if (WARN_ON_ONCE(vsie_sca->sca_o_nr_pages != nr_pages))

[Severity: High]
If the guest provides an invalid physical address for sca_gpa,
pin_guest_pages() will legitimately fail and return -EFAULT.

Because sca_o_nr_pages is an unsigned long, will this negative error
code be implicitly cast to a huge positive number?

This would cause the WARN_ON_ONCE() to evaluate to true. Does this allow
a malicious guest to intentionally trigger a host kernel warning,
leading to a denial of service if panic_on_warn is enabled?

> +		return -EIO;
> +	__set_bit(VSIE_SCA_PINNED, &vsie_sca->flags);
> +
> +	return 0;
> +}
> +
> +static void free_vsie_sca(struct kvm *kvm, struct vsie_sca *vsie_sca)
> +{
> +	free_pages_exact(vsie_sca, sizeof(*vsie_sca));

[Severity: High]
Is it possible for free_vsie_sca() to be called with a NULL pointer
during error cleanup?

Unlike kfree(), free_pages_exact() is not NULL-safe for allocations larger
than PAGE_SIZE. Since struct vsie_sca is roughly 10.5 KB, free_pages_exact()
will attempt to free invalid low-memory addresses starting at 4096 on its
second iteration, which would crash the kernel.

> +}
> +
> +static struct vsie_sca *alloc_vsie_sca(void)
> +{
> +	return alloc_pages_exact(sizeof(struct vsie_sca), GFP_KERNEL_ACCOUNT | __GFP_ZERO);

[Severity: High]
Does relying solely on __GFP_ZERO leave the embedded mutex and ref_count
uninitialized?

Locking a zero-filled mutex is a structural defect that triggers lockdep
splats or kernel panics because its internal spinlock and wait queues
are not initialized.  Should this function call mutex_init() and
atomic_set() before returning the allocated structure?

[Severity: Low]
This patch introduces alloc_vsie_sca(), free_vsie_sca(), pin_sca(), and
unpin_sca() as static functions, but they don't appear to be invoked
anywhere in this file.

Could this trigger -Wunused-function compiler warnings and break builds
that compile with -Werror?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=15