Re: [PATCH v1 07/10] KVM: s390: Fix UCONTROL inconsistencies

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: s390: _crste_ucas() collides with token encoding in _crste_hole()
--

commit 709d72ff37a22ce1d8ae00a0f230ee566c74c804
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: Fix UCONTROL inconsistencies

This commit changes the flags used in union crste to represent mapped but
unallocated UCONTROL segment entries so that they do not collide with the
present bit. It introduces _crste_ucas() and updates crste_is_ucas() and
the UCONTROL gmap helpers accordingly.

> diff --git a/arch/s390/kvm/gmap/dat.h b/arch/s390/kvm/gmap/dat.h
> index 5dceb397fc1b8..5b03a21c180da 100644
> --- a/arch/s390/kvm/gmap/dat.h
> +++ b/arch/s390/kvm/gmap/dat.h
> @@ -105,6 +105,8 @@ union pte {
>  #define _REGION3_FR_MASK	(_REGION3_MASK >> PAGE_SHIFT)
>  #define _PAGES_PER_SEGMENT	_PAGE_ENTRIES
>  #define _PAGES_PER_REGION3	(_PAGES_PER_SEGMENT * _CRST_ENTRIES)
> +#define _UCAS_ENTRY_LENGTH	0
> +#define _UCAS_ENTRY_OFFSET	3
>  
>  #define CRDTE_NODAT		IPTE_NODAT
>  #define CRDTE_GUEST_ASCE	IPTE_GUEST_ASCE
> @@ -429,6 +431,17 @@ static inline union crste _crste_fc0(kvm_pfn_t pfn, int tt)
>  	return res;
>  }
>  
> +static inline union crste _crste_ucas(kvm_pfn_t pfn)
> +{
> +	union crste res = { .val = PFN_PHYS(pfn) };
> +
> +	res.h.i = 1;
> +	res.h.tt = TABLE_TYPE_SEGMENT;
> +	res.h.fc0.tl = _UCAS_ENTRY_LENGTH;
> +	res.h.fc0.tf = _UCAS_ENTRY_OFFSET;
> +	return res;
> +}

[Severity: Medium]
Does _crste_ucas() collide with the token encoding checked by _crste_hole()
and _crste_none()?

In _crste_ucas(), res.h.i is 1 and res.h.fc0.tl is 0, which means bit 63
(res.tok.pr and res.s.fc1.pr) is 0 while bits 0..43 of res.val hold the
segment-aligned host address PFN_PHYS(pfn).

In arch/s390/kvm/gmap/dat.h, _crste_hole() and _crste_none() do not check
crste_is_ucas() or crste.h.fc0.tf:

static inline bool _crste_hole(union crste crste)
{
	return crste.h.i && !crste.tok.pr && crste.tok.type != _DAT_TOKEN_NONE;
}

static inline bool _crste_none(union crste crste)
{
	return crste.h.i && !crste.tok.pr && crste.tok.type == _DAT_TOKEN_NONE;
}

When a UCONTROL segment is mapped via KVM_S390_UCAS_MAP with a host
userspace address at or above 0x0001000000000000UL (so bits 0..15,
crste.tok.type, are non-zero) before the parent segment is faulted in,
_crste_hole() returns true for the _crste_ucas() entry.

While this patch passes DAT_WALK_IGN_HOLES in gmap_ucas_map_one(),
gmap_ucas_translate_simple(), and gmap_ucas_unmap_one(), other walkers of
vcpu->arch.gmap->datt do not. For example:

kvm_arch_vcpu_ioctl()
  -> kvm_s390_vcpu_memsida_op()
    -> kvm_s390_vcpu_mem_op()
      -> check_gva_range()
        -> guest_range_to_gpas()
          -> vcpu_check_access_key_gpa()
            -> dat_get_storage_key()

In arch/s390/kvm/gmap/dat.c:dat_get_storage_key():

	rc = dat_entry_walk(NULL, gfn, datt, DAT_WALK_ANY, TABLE_TYPE_PAGE_TABLE, &crstep, &ptep);
	if (rc)
		return rc;

And in arch/s390/kvm/gmap/dat.c:dat_entry_walk():

	if (crste_hole(entry) && !ign_holes)
		return entry.tok.type == _DAT_TOKEN_PIC ? entry.tok.par : -EFAULT;

Instead of returning storage key 0 for the unallocated segment,
dat_get_storage_key() will fail with -EFAULT (or return bits 16..31 of the
host address in entry.tok.par if entry.tok.type happens to equal
_DAT_TOKEN_PIC). Should _crste_hole() and _crste_none() exclude
crste_is_ucas(crste)?

[ ... ]

> @@ -1027,7 +1040,8 @@ static inline int dat_create_slot(struct kvm_s390_mmu_cache *mc, struct dat_tabl
>  
>  static inline bool crste_is_ucas(union crste crste)
>  {
> -	return is_pmd(crste) && crste.h.i && crste.h.fc0.tl == 1 && crste.h.fc == 0;
> +	return is_pmd(crste) && crste.h.i && !crste.h.fc &&
> +	       crste.h.fc0.tl == _UCAS_ENTRY_LENGTH && crste.h.fc0.tf == _UCAS_ENTRY_OFFSET;
>  }
>  
>  static inline void dat_set_storage_key_from_pgste(union pte pte, union pgste pgste)

[ ... ]

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