Skip to content

Commit

Permalink
i#6584: Fix buffer overflow in dr_set_sve_vector_length (#6591)
Browse files Browse the repository at this point in the history
Fixes a buffer overflow in dr_set_sve_vector_length().

Changes dr_set_sve_vector_length() to return a boolean, with a
curiosity assert so we'll know if internal uses pass incorrect values.

Updates the dr_set_sve_vector_length() test to check failure cases.

Tested in an internal build with a memory tool where the overflow was
detected:
confirmed no error is reported anymore (with the raw2trace call to
dr_set_sve_vector_length() put back to pass 0 for arm-on-x86).

Fixes: #6584
  • Loading branch information
derekbruening authored Jan 26, 2024
1 parent 5106291 commit 4f8e12f
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 10 deletions.
13 changes: 7 additions & 6 deletions core/ir/decode_shared.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* **********************************************************
* Copyright (c) 2011-2021 Google, Inc. All rights reserved.
* Copyright (c) 2011-2024 Google, Inc. All rights reserved.
* Copyright (c) 2001-2010 VMware, Inc. All rights reserved.
* **********************************************************/

Expand Down Expand Up @@ -179,17 +179,18 @@ int sve_veclen;
int sve_veclens[] = { 128, 256, 384, 512, 640, 768, 896, 1024,
1152, 1280, 1408, 1536, 1664, 1792, 1920, 2048 };

void
bool
dr_set_sve_vector_length(int vl)
{
/* TODO i#3044: Vector length will be read from h/w when running on SVE. */
for (int i = 0; i < sizeof(sve_veclens); i++) {
for (int i = 0; i < sizeof(sve_veclens) / sizeof(sve_veclens[0]); i++) {
if (vl == sve_veclens[i]) {
sve_veclen = vl;
return;
return true;
}
}
CLIENT_ASSERT(false, "invalid SVE vector length");
/* Make unusual values visible in case our internal uses mess up. */
ASSERT_CURIOSITY(false);
return false;
}

int
Expand Down
3 changes: 2 additions & 1 deletion core/ir/encode_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,11 @@ DR_API
/**
* AArch64 Scalable Vector Extension's vector length in bits is one of:
* 128 256 384 512 640 768 896 1024 1152 1280 1408 1536 1664 1792 1920 2048
* Returns whether successful.
* TODO i#3044: This function will only allow setting vector length if not
* running on SVE.
*/
void
bool
dr_set_sve_vector_length(int vl);

DR_API
Expand Down
11 changes: 8 additions & 3 deletions suite/tests/api/ir_aarch64.c
Original file line number Diff line number Diff line change
Expand Up @@ -6929,11 +6929,16 @@ test_vector_length(void *dcontext)
{
/* XXX i#6575: Add further tests. For now, make sure these are exported. */
const int new_len = 2048;
/* XXX: Probably this should return a bool so we know whether it succeeded!
* It says it fails if on actual SVE hardware: but is there an easy way to know?
/* XXX: Make this test work when on actual SVE hardware where this API routine
* is documented as failing.
*/
dr_set_sve_vector_length(new_len);
bool res = dr_set_sve_vector_length(new_len);
ASSERT(res);
ASSERT(dr_get_sve_vector_length() == new_len);
/* Ensure invalid lengths return failure. */
ASSERT(!dr_set_sve_vector_length(0));
ASSERT(!dr_set_sve_vector_length(1));
ASSERT(!dr_set_sve_vector_length(4096));
}

int
Expand Down

0 comments on commit 4f8e12f

Please sign in to comment.