Skip to content

Commit

Permalink
SDL_CalculateRGBSize, SDL_CalculateYUVSize: set the error indicator
Browse files Browse the repository at this point in the history 
These functions historically didn't set the error indicator on overflow.

Before commit 447b508 "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 <[email protected]>
  • Loading branch information
@smcv
smcv committed Jan 18, 2024
1 parent e9e53bd commit 5a6f694
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 22 deletions.
11 changes: 5 additions & 6 deletions src/video/SDL_surface.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
30 changes: 15 additions & 15 deletions src/video/SDL_yuv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -81,27 +81,27 @@ 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;
}
} else {
/* 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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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
}

Expand Down
9 changes: 8 additions & 1 deletion test/testautomation_surface.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 5a6f694

Please sign in to comment.