[FFmpeg-devel] [PATCH] checkasm/h264dsp: Fix stack-buffer-overflow, effective-type violations

* [FFmpeg-devel] [PATCH] checkasm/h264dsp: Fix stack-buffer-overflow, effective-type violations
@ 2025-06-16 10:53 Andreas Rheinhardt
  2025-06-16 15:01 ` Tristan Matthews via ffmpeg-devel
  0 siblings, 1 reply; 3+ messages in thread
From: Andreas Rheinhardt @ 2025-06-16 10:53 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

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

Patch attached.

- Andreas

[-- Attachment #2: 0001-checkasm-h264dsp-Fix-stack-buffer-overflow-effective.patch --]
[-- Type: text/x-patch, Size: 4695 bytes --]

From 06e7f9784d2a75122abe656f6b22e7042205e68a Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Mon, 16 Jun 2025 12:50:08 +0200
Subject: [PATCH] checkasm/h264dsp: Fix stack-buffer-overflow, effective-type
 violations

Also ensure that the dst buffers are not too big
(they had the right size for >8 bit depths and were therefore
too big for eight bit, letting potential buffer overflows
in the eight bit version go undetected).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 tests/checkasm/checkasm.h | 12 +++++++-----
 tests/checkasm/h264dsp.c  | 35 +++++++++++++++++++++++------------
 2 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
index e829942d58..aa2932f734 100644
--- a/tests/checkasm/checkasm.h
+++ b/tests/checkasm/checkasm.h
@@ -443,16 +443,18 @@ DECL_CHECKASM_CHECK_FUNC(int32_t);
 #define checkasm_check_pixel_padded_align(...) \
     checkasm_check_pixel2(__VA_ARGS__, 8)
 
-/* This assumes that there is a local variable named "bit_depth".
+/* This assumes that there is a local variable named "bit_depth"
+ * and that the type-specific buffers obey the name ## _BITDEPTH
+ * convention.
  * For tests that don't have that and only operate on a single
  * bitdepth, just call checkasm_check(uint8_t, ...) directly. */
 #define checkasm_check_dctcoef(buf1, stride1, buf2, stride2, ...) \
     ((bit_depth > 8) ?                                        \
-     checkasm_check(int32_t, (const int32_t*)buf1, stride1,   \
-                             (const int32_t*)buf2, stride2,   \
+     checkasm_check(int32_t, buf1 ## _32, stride1,            \
+                             buf2 ## _32, stride2,            \
                              __VA_ARGS__) :                   \
-     checkasm_check(int16_t, (const int16_t*)buf1, stride1,   \
-                             (const int16_t*)buf2, stride2,   \
+     checkasm_check(int16_t, buf1 ## _16, stride1,            \
+                             buf2 ## _16, stride2,            \
                              __VA_ARGS__))
 
 #endif /* TESTS_CHECKASM_CHECKASM_H */
diff --git a/tests/checkasm/h264dsp.c b/tests/checkasm/h264dsp.c
index f5f9650224..f05ae419fc 100644
--- a/tests/checkasm/h264dsp.c
+++ b/tests/checkasm/h264dsp.c
@@ -328,33 +328,44 @@ static void check_idct_multiple(void)
 static void check_idct_dequant(void)
 {
     static const int depths[5] = { 8, 9, 10, 12, 14 };
-    LOCAL_ALIGNED_16(int16_t, src, [16]);
-    /* Ensure dst buffers are large enough to hold dctcoefs of all bit-depths. */
-    LOCAL_ALIGNED_16(uint8_t, dst0, [16 * 16 * sizeof(int32_t)]);
-    LOCAL_ALIGNED_16(uint8_t, dst1, [16 * 16 * sizeof(int32_t)]);
-    int16_t *dst_ref = (int16_t *)dst0;
-    int16_t *dst_new = (int16_t *)dst1;
+    LOCAL_ALIGNED_16(int16_t, src16, [16]);
+    LOCAL_ALIGNED_16(int32_t, src32, [16]);
+    LOCAL_ALIGNED_16(int16_t, dst0_16, [16 * 16]);
+    LOCAL_ALIGNED_16(int16_t, dst1_16, [16 * 16]);
+    LOCAL_ALIGNED_16(int32_t, dst0_32, [16 * 16]);
+    LOCAL_ALIGNED_16(int32_t, dst1_32, [16 * 16]);
     H264DSPContext h;
     int bit_depth, i, qmul;
     declare_func_emms(AV_CPU_FLAG_MMX | AV_CPU_FLAG_SSE2, void, int16_t *output, int16_t *input, int qmul);
 
-    for (int j = 0; j < 16; j++)
-        src[j] = (rnd() % 512) - 256;
-
     qmul = rnd() % 4096;
 
     for (i = 0; i < FF_ARRAY_ELEMS(depths); i++) {
         bit_depth = depths[i];
         ff_h264dsp_init(&h, bit_depth, 1);
 
-        memset(dst0, 0, 16 * 16 * SIZEOF_COEF);
-        memset(dst1, 0, 16 * 16 * SIZEOF_COEF);
+        void *src, *dst_ref, *dst_new;
+        if (bit_depth == 8) {
+            src     = src16;
+            dst_ref = dst0_16;
+            dst_new = dst1_16;
+            for (int j = 0; j < 16; j++)
+                src16[j] = (rnd() % 512) - 256;
+        } else {
+            src     = src32;
+            dst_ref = dst0_32;
+            dst_new = dst1_32;
+            for (int j = 0; j < 16; j++)
+                src32[j] = (rnd() % (1 << (bit_depth + 1))) - (1 << bit_depth);
+        }
+        memset(dst_ref, 0, 16 * 16 * SIZEOF_COEF);
+        memset(dst_new, 0, 16 * 16 * SIZEOF_COEF);
 
         if (check_func(h.h264_luma_dc_dequant_idct, "h264_luma_dc_dequant_idct_%d", bit_depth)) {
 
             call_ref(dst_ref, src, qmul);
             call_new(dst_new, src, qmul);
-            checkasm_check_dctcoef(dst_ref, 16*SIZEOF_COEF, dst_new, 16*SIZEOF_COEF, 16, 16, "dst");
+            checkasm_check_dctcoef(dst0, 16*SIZEOF_COEF, dst1, 16*SIZEOF_COEF, 16, 16, "dst");
             bench_new(dst_new, src, qmul);
         }
     }
-- 
2.45.2


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 3+ messages in thread