Re: [PATCH 1/2] KVM: s390: fix ESSA addressing exception injection in migration mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] kvm: s390: invalid exception code injected due to negative argument
- [High] s390/kvm: Incorrect exception masking and invalid PIC injection in __do_essa
--

commit 0857e0bc0073ed5469250557e895e66a7a5d870f
Author: Gautam Gala <ggala@linux.ibm.com>

    KVM: s390: fix ESSA addressing exception injection in migration mode
    
    __do_essa() treated any negative return from dat_perform_essa() as the
    normal "no CBRL entry appended" case and returned 0, so a page-table
    walk failure silently dropped the addressing exception.

> diff --git a/arch/s390/kvm/gmap/dat.c b/arch/s390/kvm/gmap/dat.c
> index ff80d02c9f564..1bc6d2ec5b083 100644
> --- a/arch/s390/kvm/gmap/dat.c
> +++ b/arch/s390/kvm/gmap/dat.c

[ ... ]

> @@ -1092,7 +1093,9 @@ int dat_perform_essa(union asce asce, gfn_t gfn, int orc, union essa_state *stat
>  	union pte *ptep;
>  	int res = 0;
>  
> -	if (dat_entry_walk(NULL, gfn, asce, 0, TABLE_TYPE_PAGE_TABLE, &crstep, &ptep)) {
> +	if (dat_entry_walk(NULL, gfn, asce, DAT_WALK_ANY, TABLE_TYPE_PAGE_TABLE, &crstep, &ptep))
> +		return -PGM_ADDRESSING;

[Severity: High]
Would this mask other valid, positive Program Interruption Codes (PICs)
that dat_entry_walk() might return?

For example, if dat_entry_walk() returns PGM_SECURE_STORAGE_ACCESS from
hitting a memory hole, it appears it would be unconditionally mapped to
-PGM_ADDRESSING here.

> +	if (!ptep) {
>  		*state = (union essa_state) { .exception = 1 };
>  		return -1;
>  	}
> diff --git a/arch/s390/kvm/s390/priv.c b/arch/s390/kvm/s390/priv.c
> index b3cb2c2c3aa71..2abd1bdfdf322 100644
> --- a/arch/s390/kvm/s390/priv.c
> +++ b/arch/s390/kvm/s390/priv.c

[ ... ]

> @@ -1168,8 +1168,10 @@ static inline int __do_essa(struct kvm_vcpu *vcpu, const int orc)
>  	entries = (vcpu->arch.sie_block->cbrlo & ~PAGE_MASK) >> 3;
>  
>  	nappended = dat_perform_essa(vcpu->arch.gmap->asce, gfn, orc, &state, &dirtied);
> +	if (nappended == -PGM_ADDRESSING)
> +		return kvm_s390_inject_program_int(vcpu, -PGM_ADDRESSING);

[Severity: High]
Can passing -PGM_ADDRESSING to kvm_s390_inject_program_int() result in an
invalid program interruption code being injected?

Since the code argument of kvm_s390_inject_program_int() is an unsigned u16,
passing -5 causes an integer wraparound to 65531 (0xFFFB). This would inject
a corrupt exception state and violate the architectural ABI instead of
injecting the correct Addressing Exception (0x0005).

Regarding the test_essa_migration_pgm selftest added in a subsequent
patch, it appears it fails to catch this regression because its
exception handler pgm_handler_c() hardcodes a GUEST_SYNC(PGM_ADDRESSING)
call without actually reading the true PIC from the guest's lowcore
(__LC_PGM_INT_CODE).

>  	vcpu->run->s.regs.gprs[r1] = state.val;
> -	if (nappended < 0)
> +	if (nappended == -1)
>  		return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013144225.8885-1-ggala@linux.ibm.com?part=1