linux-kernel - Re: [PATCH] KVM: do not prepare new memslot for KVM_MR

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 10 Nov 2022 01:53:47 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Yan Zhao <yan.y.zhao@...el.com>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        pbonzini@...hat.com
Subject: Re: [PATCH] KVM: do not prepare new memslot for KVM_MR_DELETE

On Thu, Nov 10, 2022, Yan Zhao wrote:
> kvm_prepare_memory_region() is not useful for KVM_MR_DELETE,
> and each kvm_arch_prepare_memory_region() does nothing more than returning
> 0 for KVM_MR_DELETE.

This is not true, s390 has an error path that fires in the DELETE case.

	/* When we are protected, we should not change the memory slots */
	if (kvm_s390_pv_get_handle(kvm))
		return -EINVAL;

	if (change == KVM_MR_DELETE || change == KVM_MR_FLAGS_ONLY)
		return 0;

> So, just don't call into kvm_prepare_memory_region() to avoid unnecessary
> error handling for KVM_MR_DELETE.

Even if the s390 case didn't exit, I would still prefer keeping the code as is.
I agree that a "dummy" call is somewhat confusing, but skipping the arch call
for one operation is equally confusing and could easily lead to future bugs,
e.g. if the order of changes were reversed and an s390 developer wasn't aware
that s390's arch hook would be bypassed on memslot deletion.