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

Ilpo Järvinen
2024-09-30T13:16:57Z

[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]

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>

--
i.