linux-kernel - Re: [PATCH v3 08/23] KVM: s390: KVM page table management functions: allocation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251120085033.8934Bc9-hca@linux.ibm.com>
Date: Thu, 20 Nov 2025 09:50:33 +0100
From: Heiko Carstens <hca@...ux.ibm.com>
To: Claudio Imbrenda <imbrenda@...ux.ibm.com>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-s390@...r.kernel.org, borntraeger@...ibm.com,
        frankja@...ux.ibm.com, nsg@...ux.ibm.com, nrb@...ux.ibm.com,
        seiden@...ux.ibm.com, schlameuss@...ux.ibm.com, svens@...ux.ibm.com,
        agordeev@...ux.ibm.com, gor@...ux.ibm.com, david@...hat.com,
        gerald.schaefer@...ux.ibm.com
Subject: Re: [PATCH v3 08/23] KVM: s390: KVM page table management functions:
 allocation

On Thu, Nov 06, 2025 at 05:11:02PM +0100, Claudio Imbrenda wrote:
> Add page table management functions to be used for KVM guest (gmap)
> page tables.
> 
> This patch adds the boilerplate and functions for the allocation and
> deallocation of DAT tables.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@...ux.ibm.com>
> ---
>  arch/s390/kvm/Makefile     |   1 +
>  arch/s390/kvm/dat.c        | 103 +++++++++++++++++++++++++++++++++++++
>  arch/s390/kvm/dat.h        |  77 +++++++++++++++++++++++++++
>  arch/s390/mm/page-states.c |   1 +
>  4 files changed, 182 insertions(+)
>  create mode 100644 arch/s390/kvm/dat.c

...

> +#define GFP_KVM_S390_MMU_CACHE (GFP_ATOMIC | __GFP_ACCOUNT | __GFP_NOWARN)
> +
> +static inline struct page_table *kvm_s390_mmu_cache_alloc_pt(struct kvm_s390_mmu_cache *mc)
> +{
> +	if (mc->n_pts)
> +		return mc->pts[--mc->n_pts];
> +	return (void *)__get_free_page(GFP_KVM_S390_MMU_CACHE);
> +}
> +
> +static inline struct crst_table *kvm_s390_mmu_cache_alloc_crst(struct kvm_s390_mmu_cache *mc)
> +{
> +	if (mc->n_crsts)
> +		return mc->crsts[--mc->n_crsts];
> +	return (void *)__get_free_pages(GFP_KVM_S390_MMU_CACHE | __GFP_COMP, CRST_ALLOC_ORDER);
> +}
> +
> +static inline struct vsie_rmap *kvm_s390_mmu_cache_alloc_rmap(struct kvm_s390_mmu_cache *mc)
> +{
> +	if (mc->n_rmaps)
> +		return mc->rmaps[--mc->n_rmaps];
> +	return kzalloc(sizeof(struct vsie_rmap), GFP_KVM_S390_MMU_CACHE);
> +}

Given that the fallback allocation (cache empty), may also fail, but unlike
for other architectures also without printing any warning, this might be
difficult to debug, if there is any caller which handles the -ENOMEM case
incorrectly. Maybe it would make sense to save a calltrace (stackdepot?)
whenever that happens, so that in case a guest dies because of incorrect /
missing -ENOMEM handling you have an indication where this happened.

Otherwise you end up scanning the whole code for all users and potential bugs,
just like I did when I reviewed the new allocation paths.