From 81ac656b7c9ec0866bfa0d67e83bedbeb3c63fe9 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 18 Jan 2024 19:41:26 +0000 Subject: [PATCH] SDL_CalculateRGBSize, SDL_CalculateYUVSize: set the error indicator These functions historically didn't set the error indicator on overflow. Before commit 447b508a "error: SDL's allocators now call SDL_OutOfMemory on error", their callers would call SDL_OutOfMemory() instead, which was assumed to be close enough in meaning: "that's a silly amount of memory that would overflow size_t" is similar to "that's more memory than is available". Now that responsibility for calling SDL_OutOfMemory() has been pushed down into SDL_calloc() and friends, the functions that check for overflows might as well set more specific errors. Signed-off-by: Simon McVittie --- src/video/SDL_surface.c | 11 +++++------ src/video/SDL_yuv.c | 30 +++++++++++++++--------------- test/testautomation_surface.c | 9 ++++++++- 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/src/video/SDL_surface.c b/src/video/SDL_surface.c index 3ca8519c83..71f1159a44 100644 --- a/src/video/SDL_surface.c +++ b/src/video/SDL_surface.c @@ -47,28 +47,27 @@ static int SDL_CalculateRGBSize(Uint32 format, size_t width, size_t height, size { if (SDL_BITSPERPIXEL(format) >= 8) { if (SDL_size_mul_overflow(width, SDL_BYTESPERPIXEL(format), pitch)) { - return -1; + return SDL_SetError("width * bpp would overflow"); } } else { if (SDL_size_mul_overflow(width, SDL_BITSPERPIXEL(format), pitch)) { - return -1; + return SDL_SetError("width * bpp would overflow"); } if (SDL_size_add_overflow(*pitch, 7, pitch)) { - return -1; + return SDL_SetError("aligning pitch would overflow"); } *pitch /= 8; } if (!minimal) { /* 4-byte aligning for speed */ if (SDL_size_add_overflow(*pitch, 3, pitch)) { - return -1; + return SDL_SetError("aligning pitch would overflow"); } *pitch &= ~3; } if (SDL_size_mul_overflow(height, *pitch, size)) { - /* Overflow... */ - return -1; + return SDL_SetError("height * pitch would overflow"); } return 0; diff --git a/src/video/SDL_yuv.c b/src/video/SDL_yuv.c index 63e0236854..cd9fa8b92c 100644 --- a/src/video/SDL_yuv.c +++ b/src/video/SDL_yuv.c @@ -72,7 +72,7 @@ int SDL_CalculateYUVSize(Uint32 format, int w, int h, size_t *size, size_t *pitc /* sz_plane == w * h; */ size_t s1; if (SDL_size_mul_overflow(w, h, &s1) < 0) { - return -1; + return SDL_SetError("width * height would overflow"); } sz_plane = (int) s1; } @@ -81,15 +81,15 @@ int SDL_CalculateYUVSize(Uint32 format, int w, int h, size_t *size, size_t *pitc /* sz_plane_chroma == ((w + 1) / 2) * ((h + 1) / 2); */ size_t s1, s2, s3; if (SDL_size_add_overflow(w, 1, &s1) < 0) { - return -1; + return SDL_SetError("width + 1 would overflow"); } s1 = s1 / 2; if (SDL_size_add_overflow(h, 1, &s2) < 0) { - return -1; + return SDL_SetError("height + 1 would overflow"); } s2 = s2 / 2; if (SDL_size_mul_overflow(s1, s2, &s3) < 0) { - return -1; + return SDL_SetError("width * height would overflow"); } sz_plane_chroma = (int) s3; } @@ -97,11 +97,11 @@ int SDL_CalculateYUVSize(Uint32 format, int w, int h, size_t *size, size_t *pitc /* sz_plane_packed == ((w + 1) / 2) * h; */ size_t s1, s2; if (SDL_size_add_overflow(w, 1, &s1) < 0) { - return -1; + return SDL_SetError("width + 1 would overflow"); } s1 = s1 / 2; if (SDL_size_mul_overflow(s1, h, &s2) < 0) { - return -1; + return SDL_SetError("width * height would overflow"); } sz_plane_packed = (int) s2; } @@ -118,10 +118,10 @@ int SDL_CalculateYUVSize(Uint32 format, int w, int h, size_t *size, size_t *pitc /* dst_size == sz_plane + sz_plane_chroma + sz_plane_chroma; */ size_t s1, s2; if (SDL_size_add_overflow(sz_plane, sz_plane_chroma, &s1) < 0) { - return -1; + return SDL_SetError("Y + U would overflow"); } if (SDL_size_add_overflow(s1, sz_plane_chroma, &s2) < 0) { - return -1; + return SDL_SetError("Y + U + V would overflow"); } *size = (int)s2; } @@ -135,11 +135,11 @@ int SDL_CalculateYUVSize(Uint32 format, int w, int h, size_t *size, size_t *pitc /* pitch == ((w + 1) / 2) * 4; */ size_t p1, p2; if (SDL_size_add_overflow(w, 1, &p1) < 0) { - return -1; + return SDL_SetError("width + 1 would overflow"); } p1 = p1 / 2; if (SDL_size_mul_overflow(p1, 4, &p2) < 0) { - return -1; + return SDL_SetError("width * 4 would overflow"); } *pitch = p2; } @@ -148,7 +148,7 @@ int SDL_CalculateYUVSize(Uint32 format, int w, int h, size_t *size, size_t *pitc /* dst_size == 4 * sz_plane_packed; */ size_t s1; if (SDL_size_mul_overflow(sz_plane_packed, 4, &s1) < 0) { - return -1; + return SDL_SetError("plane * 4 would overflow"); } *size = (int) s1; } @@ -164,22 +164,22 @@ int SDL_CalculateYUVSize(Uint32 format, int w, int h, size_t *size, size_t *pitc /* dst_size == sz_plane + sz_plane_chroma + sz_plane_chroma; */ size_t s1, s2; if (SDL_size_add_overflow(sz_plane, sz_plane_chroma, &s1) < 0) { - return -1; + return SDL_SetError("Y + U would overflow"); } if (SDL_size_add_overflow(s1, sz_plane_chroma, &s2) < 0) { - return -1; + return SDL_SetError("Y + U + V would overflow"); } *size = (int) s2; } break; default: - return -1; + return SDL_Unsupported(); } return 0; #else - return -1; + return SDL_Unsupported(); #endif } diff --git a/test/testautomation_surface.c b/test/testautomation_surface.c index 3405ae3000..def9c35f03 100644 --- a/test/testautomation_surface.c +++ b/test/testautomation_surface.c @@ -757,22 +757,29 @@ static int surface_testOverflow(void *arg) "Expected \"%s\", got \"%s\"", expectedError, SDL_GetError()); if (sizeof(size_t) == 4 && sizeof(int) >= 4) { - expectedError = "Out of memory"; + SDL_ClearError(); + expectedError = "aligning pitch would overflow"; /* 0x5555'5555 * 3bpp = 0xffff'ffff which fits in size_t, but adding * alignment padding makes it overflow */ surface = SDL_CreateSurface(0x55555555, 1, SDL_PIXELFORMAT_RGB24); SDLTest_AssertCheck(surface == NULL, "Should detect overflow in pitch + alignment"); SDLTest_AssertCheck(SDL_strcmp(SDL_GetError(), expectedError) == 0, "Expected \"%s\", got \"%s\"", expectedError, SDL_GetError()); + SDL_ClearError(); + expectedError = "width * bpp would overflow"; /* 0x4000'0000 * 4bpp = 0x1'0000'0000 which (just) overflows */ surface = SDL_CreateSurface(0x40000000, 1, SDL_PIXELFORMAT_ARGB8888); SDLTest_AssertCheck(surface == NULL, "Should detect overflow in width * bytes per pixel"); SDLTest_AssertCheck(SDL_strcmp(SDL_GetError(), expectedError) == 0, "Expected \"%s\", got \"%s\"", expectedError, SDL_GetError()); + SDL_ClearError(); + expectedError = "height * pitch would overflow"; surface = SDL_CreateSurface((1 << 29) - 1, (1 << 29) - 1, SDL_PIXELFORMAT_INDEX8); SDLTest_AssertCheck(surface == NULL, "Should detect overflow in width * height"); SDLTest_AssertCheck(SDL_strcmp(SDL_GetError(), expectedError) == 0, "Expected \"%s\", got \"%s\"", expectedError, SDL_GetError()); + SDL_ClearError(); + expectedError = "height * pitch would overflow"; surface = SDL_CreateSurface((1 << 15) + 1, (1 << 15) + 1, SDL_PIXELFORMAT_ARGB8888); SDLTest_AssertCheck(surface == NULL, "Should detect overflow in width * height * bytes per pixel"); SDLTest_AssertCheck(SDL_strcmp(SDL_GetError(), expectedError) == 0,