diff mbox series

[V2,03/13] selftests/resctrl: Fix memory overflow due to unhandled wraparound

Message ID dc99091aeadca217f297c4cc0d4015bcb80443ad.1726164080.git.reinette.chatre@intel.com
State Superseded
Headers show
Series selftests/resctrl: Support diverse platforms with MBM and MBA tests | expand

Commit Message

Reinette Chatre Sept. 12, 2024, 6:13 p.m. UTC
alloc_buffer() allocates and initializes (with random data) a
buffer of requested size. The initialization starts from the beginning
of the allocated buffer and incrementally assigns sizeof(uint64_t) random
data to each cache line. The initialization uses the size of the
buffer to control the initialization flow, decrementing the amount of
buffer needing to be initialized after each iteration.

The size of the buffer is stored in an unsigned (size_t) variable s64
and the test "s64 > 0" is used to decide if initialization is complete.
The problem is that decrementing the buffer size may wrap around
if the buffer size is not divisible by "CL_SIZE / sizeof(uint64_t)"
resulting in the "s64 > 0" test being true and memory beyond the buffer
"initialized".

Use a signed value for the buffer size to support all buffer sizes.

Fixes: a2561b12fe39 ("selftests/resctrl: Add built in benchmark")
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
---
Changes since V1:
- New patch.
---
 tools/testing/selftests/resctrl/fill_buf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ilpo Järvinen Sept. 30, 2024, 1:16 p.m. UTC | #1
On Thu, 12 Sep 2024, Reinette Chatre wrote:

> alloc_buffer() allocates and initializes (with random data) a
> buffer of requested size. The initialization starts from the beginning
> of the allocated buffer and incrementally assigns sizeof(uint64_t) random
> data to each cache line. The initialization uses the size of the
> buffer to control the initialization flow, decrementing the amount of
> buffer needing to be initialized after each iteration.
> 
> The size of the buffer is stored in an unsigned (size_t) variable s64
> and the test "s64 > 0" is used to decide if initialization is complete.
> The problem is that decrementing the buffer size may wrap around
> if the buffer size is not divisible by "CL_SIZE / sizeof(uint64_t)"
> resulting in the "s64 > 0" test being true and memory beyond the buffer
> "initialized".
> 
> Use a signed value for the buffer size to support all buffer sizes.
> 
> Fixes: a2561b12fe39 ("selftests/resctrl: Add built in benchmark")
> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
> ---
> Changes since V1:
> - New patch.
> ---
>  tools/testing/selftests/resctrl/fill_buf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c
> index ae120f1735c0..34e5df721430 100644
> --- a/tools/testing/selftests/resctrl/fill_buf.c
> +++ b/tools/testing/selftests/resctrl/fill_buf.c
> @@ -127,7 +127,7 @@ unsigned char *alloc_buffer(size_t buf_size, int memflush)
>  {
>  	void *buf = NULL;
>  	uint64_t *p64;
> -	size_t s64;
> +	ssize_t s64;
>  	int ret;
>  
>  	ret = posix_memalign(&buf, PAGE_SIZE, buf_size);

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
diff mbox series

Patch

diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c
index ae120f1735c0..34e5df721430 100644
--- a/tools/testing/selftests/resctrl/fill_buf.c
+++ b/tools/testing/selftests/resctrl/fill_buf.c
@@ -127,7 +127,7 @@  unsigned char *alloc_buffer(size_t buf_size, int memflush)
 {
 	void *buf = NULL;
 	uint64_t *p64;
-	size_t s64;
+	ssize_t s64;
 	int ret;
 
 	ret = posix_memalign(&buf, PAGE_SIZE, buf_size);