linux-kernel - Re: [PATCH] KVM: selftests: Add 'malloc' failure check in test_vmx_nested

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]

Message-ID: <ZifMAWn32tZBQHs0@google.com>
Date: Tue, 23 Apr 2024 07:56:01 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Markus Elfring <Markus.Elfring@....de>
Cc: Kunwu Chan <chentao@...inos.cn>, linux-kselftest@...r.kernel.org, 
	kvm@...r.kernel.org, kernel-janitors@...r.kernel.org, 
	Muhammad Usama Anjum <usama.anjum@...labora.com>, Paolo Bonzini <pbonzini@...hat.com>, 
	Shuah Khan <shuah@...nel.org>, LKML <linux-kernel@...r.kernel.org>, 
	Kunwu Chan <kunwu.chan@...mail.com>, Andrew Jones <ajones@...tanamicro.com>, 
	Anup Patel <anup@...infault.org>, Thomas Huth <thuth@...hat.com>, 
	Oliver Upton <oliver.upton@...ux.dev>
Subject: Re: [PATCH] KVM: selftests: Add 'malloc' failure check in test_vmx_nested_state

+others

On Tue, Apr 23, 2024, Markus Elfring wrote:
> …
> > This patch will add the malloc failure checking
> …
> 
> * Please use a corresponding imperative wording for the change description.
> 
> * Would you like to add the tag “Fixes” accordingly?

Nah, don't bother with Fixes.  OOM will cause the test to fail regardless, the
fact that it gets an assert instead a NULL pointer deref is nice to have, but by
no means does it fix a bug.

> > +++ b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
> > @@ -91,6 +91,7 @@ void test_vmx_nested_state(struct kvm_vcpu *vcpu)
> >  	const int state_sz = sizeof(struct kvm_nested_state) + getpagesize();
> >  	struct kvm_nested_state *state =
> >  		(struct kvm_nested_state *)malloc(state_sz);
> > +	TEST_ASSERT(state, "-ENOMEM when allocating kvm state");
> …
> 
> Can “errno” be relevant for the error message construction?

Probably not, but there's also no reason to assume ENOMEM.  TEST_ASSERT() spits
out the actual errno, and we can just say something like "malloc() failed for
blah blah blah".  

But rather than keeping playing whack-a-mole, what if we add macros to perform
allocations and assert on the result?  I have zero interest in chasing down all
of the "unsafe" allocations, and odds are very good that we'll collectively fail
to enforce checking on new code.

E.g. something like (obviously won't compile, just for demonstration purposes)

#define kvm_malloc(x)
({
	void *__ret;

	__ret  = malloc(x);
	TEST_ASSERT(__ret, "Failed malloc(" #x ")\n");
	__ret;
})

#define kvm_calloc(x, y)
({
	void *__ret;

	__ret  = calloc(x, y);
	TEST_ASSERT(__ret, "Failed calloc(" #x ", " #y ")\n");
	__ret;
})