| 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
| ||
|
Message-Id: <20220420173513.1217360-5-bgardon@google.com>
Date: Wed, 20 Apr 2022 10:35:07 -0700
From: Ben Gardon <bgardon@...gle.com>
To: linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Cc: Paolo Bonzini <pbonzini@...hat.com>, Peter Xu <peterx@...hat.com>,
Sean Christopherson <seanjc@...gle.com>,
David Matlack <dmatlack@...gle.com>,
Jim Mattson <jmattson@...gle.com>,
David Dunn <daviddunn@...gle.com>,
Jing Zhang <jingzhangos@...gle.com>,
Junaid Shahid <junaids@...gle.com>,
Ben Gardon <bgardon@...gle.com>
Subject: [PATCH v6 04/10] KVM: selftests: Clean up coding style in binary
stats test
From: Sean Christopherson <seanjc@...gle.com>
Fix a variety of code style violations and/or inconsistencies in the
binary stats test. The 80 char limit is a soft limit and can and should
be ignored/violated if doing so improves the overall code readability.
Specifically, provide consistent indentation and don't split expressions
at arbitrary points just to honor the 80 char limit.
Opportunistically expand/add comments to call out the more subtle aspects
of the code.
Signed-off-by: Sean Christopherson <seanjc@...gle.com>
Reviewed-by: David Matlack <dmatlack@...gle.com>
Signed-off-by: Ben Gardon <bgardon@...gle.com>
---
.../selftests/kvm/kvm_binary_stats_test.c | 91 ++++++++++++-------
1 file changed, 56 insertions(+), 35 deletions(-)
diff --git a/tools/testing/selftests/kvm/kvm_binary_stats_test.c b/tools/testing/selftests/kvm/kvm_binary_stats_test.c
index b49fae45db1e..8b31f8fc7e08 100644
--- a/tools/testing/selftests/kvm/kvm_binary_stats_test.c
+++ b/tools/testing/selftests/kvm/kvm_binary_stats_test.c
@@ -35,47 +35,64 @@ static void stats_test(int stats_fd)
/* Read kvm stats header */
read_stats_header(stats_fd, &header);
+ /*
+ * The base size of the descriptor is defined by KVM's ABI, but the
+ * size of the name field is variable as far as KVM's ABI is concerned.
+ * But, the size of name is constant for a given instance of KVM and
+ * is provided by KVM in the overall stats header.
+ */
size_desc = sizeof(*stats_desc) + header.name_size;
/* Read kvm stats id string */
id = malloc(header.name_size);
TEST_ASSERT(id, "Allocate memory for id string");
+
ret = read(stats_fd, id, header.name_size);
TEST_ASSERT(ret == header.name_size, "Read id string");
/* Check id string, that should start with "kvm" */
TEST_ASSERT(!strncmp(id, "kvm", 3) && strlen(id) < header.name_size,
- "Invalid KVM stats type, id: %s", id);
+ "Invalid KVM stats type, id: %s", id);
/* Sanity check for other fields in header */
if (header.num_desc == 0) {
printf("No KVM stats defined!");
return;
}
- /* Check overlap */
- TEST_ASSERT(header.desc_offset > 0 && header.data_offset > 0
- && header.desc_offset >= sizeof(header)
- && header.data_offset >= sizeof(header),
- "Invalid offset fields in header");
+ /*
+ * The descriptor and data offsets must be valid, they must not overlap
+ * the header, and the descriptor and data blocks must not overlap each
+ * other. Note, the data block is rechecked after its size is known.
+ */
+ TEST_ASSERT(header.desc_offset && header.desc_offset >= sizeof(header) &&
+ header.data_offset && header.data_offset >= sizeof(header),
+ "Invalid offset fields in header");
+
TEST_ASSERT(header.desc_offset > header.data_offset ||
- (header.desc_offset + size_desc * header.num_desc <=
- header.data_offset),
- "Descriptor block is overlapped with data block");
+ (header.desc_offset + size_desc * header.num_desc <= header.data_offset),
+ "Descriptor block is overlapped with data block");
/* Read kvm stats descriptors */
stats_desc = read_stats_desc(stats_fd, &header);
/* Sanity check for fields in descriptors */
for (i = 0; i < header.num_desc; ++i) {
+ /*
+ * Note, size_desc includes the of the name field, which is
+ * variable, i.e. this is NOT equivalent to &stats_desc[i].
+ */
pdesc = (void *)stats_desc + i * size_desc;
- /* Check type,unit,base boundaries */
- TEST_ASSERT((pdesc->flags & KVM_STATS_TYPE_MASK)
- <= KVM_STATS_TYPE_MAX, "Unknown KVM stats type");
- TEST_ASSERT((pdesc->flags & KVM_STATS_UNIT_MASK)
- <= KVM_STATS_UNIT_MAX, "Unknown KVM stats unit");
- TEST_ASSERT((pdesc->flags & KVM_STATS_BASE_MASK)
- <= KVM_STATS_BASE_MAX, "Unknown KVM stats base");
- /* Check exponent for stats unit
+
+ /* Check type, unit, and base boundaries */
+ TEST_ASSERT((pdesc->flags & KVM_STATS_TYPE_MASK) <= KVM_STATS_TYPE_MAX,
+ "Unknown KVM stats type");
+ TEST_ASSERT((pdesc->flags & KVM_STATS_UNIT_MASK) <= KVM_STATS_UNIT_MAX,
+ "Unknown KVM stats unit");
+ TEST_ASSERT((pdesc->flags & KVM_STATS_BASE_MASK) <= KVM_STATS_BASE_MAX,
+ "Unknown KVM stats base");
+
+ /*
+ * Check exponent for stats unit
* Exponent for counter should be greater than or equal to 0
* Exponent for unit bytes should be greater than or equal to 0
* Exponent for unit seconds should be less than or equal to 0
@@ -86,47 +103,51 @@ static void stats_test(int stats_fd)
case KVM_STATS_UNIT_NONE:
case KVM_STATS_UNIT_BYTES:
case KVM_STATS_UNIT_CYCLES:
- TEST_ASSERT(pdesc->exponent >= 0,
- "Unsupported KVM stats unit");
+ TEST_ASSERT(pdesc->exponent >= 0, "Unsupported KVM stats unit");
break;
case KVM_STATS_UNIT_SECONDS:
- TEST_ASSERT(pdesc->exponent <= 0,
- "Unsupported KVM stats unit");
+ TEST_ASSERT(pdesc->exponent <= 0, "Unsupported KVM stats unit");
break;
}
/* Check name string */
TEST_ASSERT(strlen(pdesc->name) < header.name_size,
- "KVM stats name(%s) too long", pdesc->name);
+ "KVM stats name(%s) too long", pdesc->name);
/* Check size field, which should not be zero */
- TEST_ASSERT(pdesc->size, "KVM descriptor(%s) with size of 0",
- pdesc->name);
+ TEST_ASSERT(pdesc->size,
+ "KVM descriptor(%s) with size of 0", pdesc->name);
/* Check bucket_size field */
switch (pdesc->flags & KVM_STATS_TYPE_MASK) {
case KVM_STATS_TYPE_LINEAR_HIST:
TEST_ASSERT(pdesc->bucket_size,
- "Bucket size of Linear Histogram stats (%s) is zero",
- pdesc->name);
+ "Bucket size of Linear Histogram stats (%s) is zero",
+ pdesc->name);
break;
default:
TEST_ASSERT(!pdesc->bucket_size,
- "Bucket size of stats (%s) is not zero",
- pdesc->name);
+ "Bucket size of stats (%s) is not zero",
+ pdesc->name);
}
size_data += pdesc->size * sizeof(*stats_data);
}
- /* Check overlap */
- TEST_ASSERT(header.data_offset >= header.desc_offset
- || header.data_offset + size_data <= header.desc_offset,
- "Data block is overlapped with Descriptor block");
+
+ /*
+ * Now that the size of the data block is known, verify the data block
+ * doesn't overlap the descriptor block.
+ */
+ TEST_ASSERT(header.data_offset >= header.desc_offset ||
+ header.data_offset + size_data <= header.desc_offset,
+ "Data block is overlapped with Descriptor block");
+
/* Check validity of all stats data size */
TEST_ASSERT(size_data >= header.num_desc * sizeof(*stats_data),
- "Data size is not correct");
+ "Data size is not correct");
+
/* Check stats offset */
for (i = 0; i < header.num_desc; ++i) {
pdesc = (void *)stats_desc + i * size_desc;
TEST_ASSERT(pdesc->offset < size_data,
- "Invalid offset (%u) for stats: %s",
- pdesc->offset, pdesc->name);
+ "Invalid offset (%u) for stats: %s",
+ pdesc->offset, pdesc->name);
}
/* Allocate memory for stats data */
--
2.36.0.rc0.470.gd361397f0d-goog
Powered by blists - more mailing lists