linux-kernel - [PATCH] KVM: selftests: Fix dynamic generation of configuration names

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

Date:   Wed, 13 Dec 2023 07:26:36 -0500
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Cc:     Andrew Jones <ajones@...tanamicro.com>,
        Mark Brown <broonie@...nel.org>
Subject: [PATCH] KVM: selftests: Fix dynamic generation of configuration names

When we dynamically generate a name for a configuration in get-reg-list
we use strcat() to append to a buffer allocated using malloc() but we
never initialise that buffer. Since malloc() offers no guarantees
regarding the contents of the memory it returns this can lead to us
corrupting, and likely overflowing, the buffer:

  vregs: PASS
  vregs+pmu: PASS
  sve: PASS
  sve+pmu: PASS
  vregs+pauth_address+pauth_generic: PASS
  X?vr+gspauth_addre+spauth_generi+pmu: PASS

The bug is that strcat() should have been strcpy(), and that replacement
would be enough to fix it, but there are other things in the function
that leave something to be desired.  In particular, an (incorrectly)
empty config would cause an out of bounds access to c->name[-1].
Since the strcpy() call relies on c->name[0..len-1] being initialized,
enforce that invariant throughout the function.

Fixes: 2f9ace5d4557 ("KVM: arm64: selftests: get-reg-list: Introduce vcpu configs")
Reviewed-by: Andrew Jones <ajones@...tanamicro.com>
Co-developed-by: Mark Brown <broonie@...nel.org>
Signed-off-by: Mark Brown <broonie@...nel.org>
Message-Id: <20231211-kvm-get-reg-list-str-init-v3-1-6554c71c77b1@...nel.org>
Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
---
 tools/testing/selftests/kvm/get-reg-list.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kvm/get-reg-list.c b/tools/testing/selftests/kvm/get-reg-list.c
index be7bf5224434..8274ef04301f 100644
--- a/tools/testing/selftests/kvm/get-reg-list.c
+++ b/tools/testing/selftests/kvm/get-reg-list.c
@@ -71,11 +71,12 @@ static const char *config_name(struct vcpu_reg_list *c)
 	for_each_sublist(c, s) {
 		if (!strcmp(s->name, "base"))
 			continue;
-		strcat(c->name + len, s->name);
-		len += strlen(s->name) + 1;
-		c->name[len - 1] = '+';
+		if (len)
+			c->name[len++] = '+';
+		strcpy(c->name + len, s->name);
+		len += strlen(s->name);
 	}
-	c->name[len - 1] = '\0';
+	c->name[len] = '\0';

 	return c->name;
 }
-- 
2.39.1