diff mbox series

[FFmpeg-devel] avcodec/ppc/vp8dsp_altivec: Fix out-of-bounds access

Message ID AS8P250MB0744152CD37F47F2F0E79D6E8F2A2@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM
State Accepted
Commit 09e6840cf7a3ee07a73c3ae88a020bf27ca1a667
Headers show
Series [FFmpeg-devel] avcodec/ppc/vp8dsp_altivec: Fix out-of-bounds access | expand

Checks

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

Commit Message

Andreas Rheinhardt March 13, 2024, 1:14 a.m. UTC
h_subpel_filters_inner[i] and h_subpel_filters_outer[i / 2]
belong together and the former allows the range 0..6,
so the latter needs to support 0..3. But it has only three
elements. Add another one.
The value for the last element has been guesstimated
from subpel_filters in libavcodec/vp8dsp.c.

This is also intended to fix FATE-failures with UBSan here:
https://fate.ffmpeg.org/report.cgi?time=20240312011016&slot=ppc-linux-gcc-13.2-ubsan-altivec-qemu

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/ppc/vp8dsp_altivec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Sean McGovern March 14, 2024, 9:18 p.m. UTC | #1
Andreas:

On Tue, Mar 12, 2024 at 9:14 PM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> h_subpel_filters_inner[i] and h_subpel_filters_outer[i / 2]
> belong together and the former allows the range 0..6,
> so the latter needs to support 0..3. But it has only three
> elements. Add another one.
> The value for the last element has been guesstimated
> from subpel_filters in libavcodec/vp8dsp.c.
>
> This is also intended to fix FATE-failures with UBSan here:
> https://fate.ffmpeg.org/report.cgi?time=20240312011016&slot=ppc-linux-gcc-13.2-ubsan-altivec-qemu
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/ppc/vp8dsp_altivec.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/ppc/vp8dsp_altivec.c b/libavcodec/ppc/vp8dsp_altivec.c
> index 12dac8b0a8..061914fc38 100644
> --- a/libavcodec/ppc/vp8dsp_altivec.c
> +++ b/libavcodec/ppc/vp8dsp_altivec.c
> @@ -50,11 +50,12 @@ static const vec_s8 h_subpel_filters_inner[7] =
>  // for 6tap filters, these are the outer two taps
>  // The zeros mask off pixels 4-7 when filtering 0-3
>  // and vice-versa
> -static const vec_s8 h_subpel_filters_outer[3] =
> +static const vec_s8 h_subpel_filters_outer[4] =
>  {
>      REPT4(0, 0, 2, 1),
>      REPT4(0, 0, 3, 3),
>      REPT4(0, 0, 1, 2),
> +    REPT4(0, 0, 0, 0),
>  };
>
>  #define LOAD_H_SUBPEL_FILTER(i) \
> --
> 2.40.1
>
> _______________________________________________
> 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".

Confirming this patch fixes fate-checkasm-vp8dsp (and presumably the
other vp8 tests in GCC UBsan) on PowerPC QEMU, POWER7 (ppc64), and
POWER9 (ppc64le).

Thanks again,
Sean McGovern
diff mbox series

Patch

diff --git a/libavcodec/ppc/vp8dsp_altivec.c b/libavcodec/ppc/vp8dsp_altivec.c
index 12dac8b0a8..061914fc38 100644
--- a/libavcodec/ppc/vp8dsp_altivec.c
+++ b/libavcodec/ppc/vp8dsp_altivec.c
@@ -50,11 +50,12 @@  static const vec_s8 h_subpel_filters_inner[7] =
 // for 6tap filters, these are the outer two taps
 // The zeros mask off pixels 4-7 when filtering 0-3
 // and vice-versa
-static const vec_s8 h_subpel_filters_outer[3] =
+static const vec_s8 h_subpel_filters_outer[4] =
 {
     REPT4(0, 0, 2, 1),
     REPT4(0, 0, 3, 3),
     REPT4(0, 0, 1, 2),
+    REPT4(0, 0, 0, 0),
 };
 
 #define LOAD_H_SUBPEL_FILTER(i) \