diff mbox series

[FFmpeg-devel,PATCHv3] swscale: prevent undefined behaviour in the PUTRGBA macro

Message ID 20240709183434.10114-1-gseanmcg@gmail.com
State New
Headers show
Series [FFmpeg-devel,PATCHv3] swscale: prevent undefined behaviour in the PUTRGBA macro | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 fail Make fate failed
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Sean McGovern July 9, 2024, 6:34 p.m. UTC 
For even small values of 'asrc', shifting them by 24 bits or more
will cause arithmetic overflow and be caught by
GCC's undefined behaviour sanitizer.

Ensure the values do not overflow by up-casting the bracketed
expressions involving 'asrc' to int32_t.
---
 libswscale/yuv2rgb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Leo Izen July 9, 2024, 8:19 p.m. UTC | #1 
On 7/9/24 2:34 PM, Sean McGovern wrote:
> For even small values of 'asrc', shifting them by 24 bits or more
> will cause arithmetic overflow and be caught by
> GCC's undefined behaviour sanitizer.
> 
> Ensure the values do not overflow by up-casting the bracketed
> expressions involving 'asrc' to int32_t.
> ---
>   libswscale/yuv2rgb.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libswscale/yuv2rgb.c b/libswscale/yuv2rgb.c
> index 977eb3a7dd..ac0b811f61 100644
> --- a/libswscale/yuv2rgb.c
> +++ b/libswscale/yuv2rgb.c
> @@ -100,9 +100,9 @@ const int *sws_getCoefficients(int colorspace)
>   
>   #define PUTRGBA(dst, ysrc, asrc, i, abase)                              \
>       Y              = ysrc[2 * i];                                       \
> -    dst[2 * i]     = r[Y] + g[Y] + b[Y] + (asrc[2 * i]     << abase);   \
> +    dst[2 * i]     = r[Y] + g[Y] + b[Y] + ((int32_t)(asrc[2 * i])     << abase);   \
>       Y              = ysrc[2 * i + 1];                                   \
> -    dst[2 * i + 1] = r[Y] + g[Y] + b[Y] + (asrc[2 * i + 1] << abase);
> +    dst[2 * i + 1] = r[Y] + g[Y] + b[Y] + ((int32_t)(asrc[2 * i + 1]) << abase);
>   
>   #define PUTRGB48(dst, src, asrc, i, abase)          \
>       Y                = src[ 2 * i];                 \

Are these able to be negative? If not, it may be wise to cast to 
uint32_t instead as left shifting a negative integer is UB.

- Leo Izen (Traneptora)
diff mbox series

Patch

diff --git a/libswscale/yuv2rgb.c b/libswscale/yuv2rgb.c
index 977eb3a7dd..ac0b811f61 100644
--- a/libswscale/yuv2rgb.c
+++ b/libswscale/yuv2rgb.c
@@ -100,9 +100,9 @@  const int *sws_getCoefficients(int colorspace)
 
 #define PUTRGBA(dst, ysrc, asrc, i, abase)                              \
     Y              = ysrc[2 * i];                                       \
-    dst[2 * i]     = r[Y] + g[Y] + b[Y] + (asrc[2 * i]     << abase);   \
+    dst[2 * i]     = r[Y] + g[Y] + b[Y] + ((int32_t)(asrc[2 * i])     << abase);   \
     Y              = ysrc[2 * i + 1];                                   \
-    dst[2 * i + 1] = r[Y] + g[Y] + b[Y] + (asrc[2 * i + 1] << abase);
+    dst[2 * i + 1] = r[Y] + g[Y] + b[Y] + ((int32_t)(asrc[2 * i + 1]) << abase);
 
 #define PUTRGB48(dst, src, asrc, i, abase)          \
     Y                = src[ 2 * i];                 \