diff mbox series

[FFmpeg-devel,V1,2/4] lavc/hevcdec: fix the HEVC decoder crash when memory over-read

Message ID 1585556574-31762-2-git-send-email-mypopydev@gmail.com
State Accepted
Commit cacdac819ff739522f32323e9394ddbe58efddc8
Headers show
Series [FFmpeg-devel,V1,1/4] lavf/flvdec: set AVFMT_TS_DISCONT flag on FLV demuxer | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork fail Make fate failed

Commit Message

Jun Zhao March 30, 2020, 8:22 a.m. UTC 
From: qoroliang <qoroliang@tencent.com>

Fix an occasional crash for hevc decoder in ARM 64 platform, the
root cause is the memory over read(read cross the memory boundary)
in SAO NENO functions ff_hevc_sao_band_filter_neon_8 and
ff_hevc_sao_edge_filter_neon_8.

After this fix, the crash disapper in the massive Android phone
test.

Signed-off-by: qoroliang <qoroliang@tencent.com>
---
 libavcodec/arm/hevcdsp_sao_neon.S | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

Comments

Jun Zhao March 30, 2020, 8:52 a.m. UTC | #1 
On Mon, Mar 30, 2020 at 4:31 PM Jun Zhao <mypopydev@gmail.com> wrote:
>
> From: qoroliang <qoroliang@tencent.com>
>
> Fix an occasional crash for hevc decoder in ARM 64 platform, the
                          typo: it's ARM 32 bits platform, not 64,
fixed in local
> root cause is the memory over read(read cross the memory boundary)
> in SAO NENO functions ff_hevc_sao_band_filter_neon_8 and
> ff_hevc_sao_edge_filter_neon_8.
>
> After this fix, the crash disapper in the massive Android phone
> test.
>
> Signed-off-by: qoroliang <qoroliang@tencent.com>
> ---
>  libavcodec/arm/hevcdsp_sao_neon.S | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/libavcodec/arm/hevcdsp_sao_neon.S b/libavcodec/arm/hevcdsp_sao_neon.S
> index 3471679..8fd9d1e 100644
> --- a/libavcodec/arm/hevcdsp_sao_neon.S
> +++ b/libavcodec/arm/hevcdsp_sao_neon.S
> @@ -35,10 +35,10 @@ function ff_hevc_sao_band_filter_neon_8, export=1
>          vmov.u16    q15,  #1
>          vmov.u8     q14,  #32
>  0:      pld      [r1]
> -        vld1.8   {d16},  [r1], r3
>          cmp      r5,    #4
>          beq      4f
>  8:      subs     r4,    #1
> +        vld1.8   {d16},  [r1], r3
>          vshr.u8  d17,   d16,  #3   // index = [src>>3]
>          vshll.u8 q9,    d17,  #1   // lowIndex = 2*index
>          vadd.u16 q11,   q9,   q15  // highIndex = (2*index+1) << 8
> @@ -54,7 +54,6 @@ function ff_hevc_sao_band_filter_neon_8, export=1
>          vaddw.u8 q13,   q12,       d16
>          vqmovun.s16      d8,         q13
>          vst1.8    d8,   [r0],      r2
> -        vld1.8   {d16}, [r1],      r3
>          bne      8b
>          subs     r5,    #8
>          beq      99f
> @@ -65,6 +64,7 @@ function ff_hevc_sao_band_filter_neon_8, export=1
>          mov r1, r7
>          b        0b
>  4:      subs     r4,    #1
> +        vld1.32   {d16[0]},  [r1],  r3
>          vshr.u8  d17,   d16,  #3  // src>>3
>          vshll.u8 q9,    d17,  #1   // lowIndex = 2*index
>          vadd.u16 q11,   q9,   q15  // highIndex = (2*index+1) << 8
> @@ -80,7 +80,6 @@ function ff_hevc_sao_band_filter_neon_8, export=1
>          vaddw.u8 q13,   q12,       d16
>          vqmovun.s16     d14,       q13
>          vst1.32   d14[0],    [r0],     r2
> -        vld1.32   {d16[0]},  [r1],     r3
>          bne      4b
>          b        99f
>  99:
> @@ -110,12 +109,12 @@ function ff_hevc_sao_edge_filter_neon_8, export=1
>          mov      r11,    r1
>          add      r11,    r9           // src[x + b_stride]
>          pld      [r1]
> -        vld1.8   {d16},  [r1],  r3    // src[x]  8x8bit
> -        vld1.8   {d17},  [r10], r3    // src[x + a_stride]
> -        vld1.8   {d18},  [r11], r3    // src[x + b_stride]
>          cmp      r5,     #4
>          beq      4f
>  8:      subs     r4,     #1
> +        vld1.8   {d16},  [r1],  r3    // src[x]  8x8bit
> +        vld1.8   {d17},  [r10], r3    // src[x + a_stride]
> +        vld1.8   {d18},  [r11], r3    // src[x + b_stride]
>          vcgt.u8  d8,     d16,   d17
>          vshr.u8  d9,     d8,    #7
>          vclt.u8  d8,     d16,   d17
> @@ -136,9 +135,6 @@ function ff_hevc_sao_edge_filter_neon_8, export=1
>          vaddw.u8 q12,    q11,   d16
>          vqmovun.s16      d26,   q12
>          vst1.8   d26,    [r0],  r2
> -        vld1.8   {d16},  [r1],  r3    // src[x]  8x8bit
> -        vld1.8   {d17},  [r10], r3    // src[x + a_stride]
> -        vld1.8   {d18},  [r11], r3    // src[x + b_stride]
>          bne      8b
>          subs     r5,     #8
>          beq      99f
> @@ -149,6 +145,9 @@ function ff_hevc_sao_edge_filter_neon_8, export=1
>          mov      r1,     r7
>          b        0b
>  4:      subs     r4,    #1
> +        vld1.32   {d16[0]},  [r1],  r3
> +        vld1.32   {d17[0]},  [r10], r3    // src[x + a_stride]
> +        vld1.32   {d18[0]},  [r11], r3    // src[x + b_stride]
>          vcgt.u8  d8,     d16,   d17
>          vshr.u8  d9,     d8,    #7
>          vclt.u8  d8,     d16,   d17
> @@ -169,9 +168,6 @@ function ff_hevc_sao_edge_filter_neon_8, export=1
>          vaddw.u8 q12,    q11,   d16
>          vqmovun.s16      d26,   q12
>          vst1.32  d26[0], [r0],  r2
> -        vld1.32   {d16[0]},  [r1],  r3
> -        vld1.32   {d17[0]},  [r10], r3    // src[x + a_stride]
> -        vld1.32   {d18[0]},  [r11], r3    // src[x + b_stride]
>          bne      4b
>          b        99f
>  99:
> --
> 2.7.4
Jun Zhao April 11, 2020, 1:54 p.m. UTC | #2 
On Mon, Mar 30, 2020 at 4:52 PM mypopy@gmail.com <mypopy@gmail.com> wrote:
>
> On Mon, Mar 30, 2020 at 4:31 PM Jun Zhao <mypopydev@gmail.com> wrote:
> >
> > From: qoroliang <qoroliang@tencent.com>
> >
> > Fix an occasional crash for hevc decoder in ARM 64 platform, the
>                           typo: it's ARM 32 bits platform, not 64,
> fixed in local
> > root cause is the memory over read(read cross the memory boundary)
> > in SAO NENO functions ff_hevc_sao_band_filter_neon_8 and
> > ff_hevc_sao_edge_filter_neon_8.
> >
> > After this fix, the crash disapper in the massive Android phone
> > test.
> >
Ping,  this issue will lead to some crash in ARM device, so pls help
to review the fix, thx.

> > Signed-off-by: qoroliang <qoroliang@tencent.com>
> > ---
> >  libavcodec/arm/hevcdsp_sao_neon.S | 20 ++++++++------------
> >  1 file changed, 8 insertions(+), 12 deletions(-)
> >
> > diff --git a/libavcodec/arm/hevcdsp_sao_neon.S b/libavcodec/arm/hevcdsp_sao_neon.S
> > index 3471679..8fd9d1e 100644
> > --- a/libavcodec/arm/hevcdsp_sao_neon.S
> > +++ b/libavcodec/arm/hevcdsp_sao_neon.S
> > @@ -35,10 +35,10 @@ function ff_hevc_sao_band_filter_neon_8, export=1
> >          vmov.u16    q15,  #1
> >          vmov.u8     q14,  #32
> >  0:      pld      [r1]
> > -        vld1.8   {d16},  [r1], r3
> >          cmp      r5,    #4
> >          beq      4f
> >  8:      subs     r4,    #1
> > +        vld1.8   {d16},  [r1], r3
> >          vshr.u8  d17,   d16,  #3   // index = [src>>3]
> >          vshll.u8 q9,    d17,  #1   // lowIndex = 2*index
> >          vadd.u16 q11,   q9,   q15  // highIndex = (2*index+1) << 8
> > @@ -54,7 +54,6 @@ function ff_hevc_sao_band_filter_neon_8, export=1
> >          vaddw.u8 q13,   q12,       d16
> >          vqmovun.s16      d8,         q13
> >          vst1.8    d8,   [r0],      r2
> > -        vld1.8   {d16}, [r1],      r3
> >          bne      8b
> >          subs     r5,    #8
> >          beq      99f
> > @@ -65,6 +64,7 @@ function ff_hevc_sao_band_filter_neon_8, export=1
> >          mov r1, r7
> >          b        0b
> >  4:      subs     r4,    #1
> > +        vld1.32   {d16[0]},  [r1],  r3
> >          vshr.u8  d17,   d16,  #3  // src>>3
> >          vshll.u8 q9,    d17,  #1   // lowIndex = 2*index
> >          vadd.u16 q11,   q9,   q15  // highIndex = (2*index+1) << 8
> > @@ -80,7 +80,6 @@ function ff_hevc_sao_band_filter_neon_8, export=1
> >          vaddw.u8 q13,   q12,       d16
> >          vqmovun.s16     d14,       q13
> >          vst1.32   d14[0],    [r0],     r2
> > -        vld1.32   {d16[0]},  [r1],     r3
> >          bne      4b
> >          b        99f
> >  99:
> > @@ -110,12 +109,12 @@ function ff_hevc_sao_edge_filter_neon_8, export=1
> >          mov      r11,    r1
> >          add      r11,    r9           // src[x + b_stride]
> >          pld      [r1]
> > -        vld1.8   {d16},  [r1],  r3    // src[x]  8x8bit
> > -        vld1.8   {d17},  [r10], r3    // src[x + a_stride]
> > -        vld1.8   {d18},  [r11], r3    // src[x + b_stride]
> >          cmp      r5,     #4
> >          beq      4f
> >  8:      subs     r4,     #1
> > +        vld1.8   {d16},  [r1],  r3    // src[x]  8x8bit
> > +        vld1.8   {d17},  [r10], r3    // src[x + a_stride]
> > +        vld1.8   {d18},  [r11], r3    // src[x + b_stride]
> >          vcgt.u8  d8,     d16,   d17
> >          vshr.u8  d9,     d8,    #7
> >          vclt.u8  d8,     d16,   d17
> > @@ -136,9 +135,6 @@ function ff_hevc_sao_edge_filter_neon_8, export=1
> >          vaddw.u8 q12,    q11,   d16
> >          vqmovun.s16      d26,   q12
> >          vst1.8   d26,    [r0],  r2
> > -        vld1.8   {d16},  [r1],  r3    // src[x]  8x8bit
> > -        vld1.8   {d17},  [r10], r3    // src[x + a_stride]
> > -        vld1.8   {d18},  [r11], r3    // src[x + b_stride]
> >          bne      8b
> >          subs     r5,     #8
> >          beq      99f
> > @@ -149,6 +145,9 @@ function ff_hevc_sao_edge_filter_neon_8, export=1
> >          mov      r1,     r7
> >          b        0b
> >  4:      subs     r4,    #1
> > +        vld1.32   {d16[0]},  [r1],  r3
> > +        vld1.32   {d17[0]},  [r10], r3    // src[x + a_stride]
> > +        vld1.32   {d18[0]},  [r11], r3    // src[x + b_stride]
> >          vcgt.u8  d8,     d16,   d17
> >          vshr.u8  d9,     d8,    #7
> >          vclt.u8  d8,     d16,   d17
> > @@ -169,9 +168,6 @@ function ff_hevc_sao_edge_filter_neon_8, export=1
> >          vaddw.u8 q12,    q11,   d16
> >          vqmovun.s16      d26,   q12
> >          vst1.32  d26[0], [r0],  r2
> > -        vld1.32   {d16[0]},  [r1],  r3
> > -        vld1.32   {d17[0]},  [r10], r3    // src[x + a_stride]
> > -        vld1.32   {d18[0]},  [r11], r3    // src[x + b_stride]
> >          bne      4b
> >          b        99f
> >  99:
> > --
> > 2.7.4
Carl Eugen Hoyos April 11, 2020, 2:47 p.m. UTC | #3 
Am Sa., 11. Apr. 2020 um 16:24 Uhr schrieb mypopy@gmail.com <mypopy@gmail.com>:
>
> On Mon, Mar 30, 2020 at 4:52 PM mypopy@gmail.com <mypopy@gmail.com> wrote:
> >
> > On Mon, Mar 30, 2020 at 4:31 PM Jun Zhao <mypopydev@gmail.com> wrote:
> > >
> > > From: qoroliang <qoroliang@tencent.com>
> > >
> > > Fix an occasional crash for hevc decoder in ARM 64 platform, the
> >                           typo: it's ARM 32 bits platform, not 64,
> > fixed in local
> > > root cause is the memory over read(read cross the memory boundary)
> > > in SAO NENO functions ff_hevc_sao_band_filter_neon_8 and
> > > ff_hevc_sao_edge_filter_neon_8.
> > >
> > > After this fix, the crash disapper in the massive Android phone
> > > test.
> > >
> Ping,  this issue will lead to some crash in ARM device, so pls help
> to review the fix, thx.

Is there a sample that allows to reproduce?

Carl Eugen
Jun Zhao April 12, 2020, 1:18 a.m. UTC | #4 
On Sat, Apr 11, 2020 at 10:48 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
> Am Sa., 11. Apr. 2020 um 16:24 Uhr schrieb mypopy@gmail.com <mypopy@gmail.com>:
> >
> > On Mon, Mar 30, 2020 at 4:52 PM mypopy@gmail.com <mypopy@gmail.com> wrote:
> > >
> > > On Mon, Mar 30, 2020 at 4:31 PM Jun Zhao <mypopydev@gmail.com> wrote:
> > > >
> > > > From: qoroliang <qoroliang@tencent.com>
> > > >
> > > > Fix an occasional crash for hevc decoder in ARM 64 platform, the
> > >                           typo: it's ARM 32 bits platform, not 64,
> > > fixed in local
> > > > root cause is the memory over read(read cross the memory boundary)
> > > > in SAO NENO functions ff_hevc_sao_band_filter_neon_8 and
> > > > ff_hevc_sao_edge_filter_neon_8.
> > > >
> > > > After this fix, the crash disapper in the massive Android phone
> > > > test.
> > > >
> > Ping,  this issue will lead to some crash in ARM device, so pls help
> > to review the fix, thx.
>
> Is there a sample that allows to reproduce?
>

In fact, this is a probabilistic problem, we can only find this
problem in large-scale product testing, and catch the coredump in a
small number of Android mobile phones

> Carl Eugen
Jun Zhao April 20, 2020, 2:11 a.m. UTC | #5 
On Sun, Apr 12, 2020 at 9:18 AM mypopy@gmail.com <mypopy@gmail.com> wrote:
>
> On Sat, Apr 11, 2020 at 10:48 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> >
> > Am Sa., 11. Apr. 2020 um 16:24 Uhr schrieb mypopy@gmail.com <mypopy@gmail.com>:
> > >
> > > On Mon, Mar 30, 2020 at 4:52 PM mypopy@gmail.com <mypopy@gmail.com> wrote:
> > > >
> > > > On Mon, Mar 30, 2020 at 4:31 PM Jun Zhao <mypopydev@gmail.com> wrote:
> > > > >
> > > > > From: qoroliang <qoroliang@tencent.com>
> > > > >
> > > > > Fix an occasional crash for hevc decoder in ARM 64 platform, the
> > > >                           typo: it's ARM 32 bits platform, not 64,
> > > > fixed in local
> > > > > root cause is the memory over read(read cross the memory boundary)
> > > > > in SAO NENO functions ff_hevc_sao_band_filter_neon_8 and
> > > > > ff_hevc_sao_edge_filter_neon_8.
> > > > >
> > > > > After this fix, the crash disapper in the massive Android phone
> > > > > test.
> > > > >
> > > Ping,  this issue will lead to some crash in ARM device, so pls help
> > > to review the fix, thx.
> >
> > Is there a sample that allows to reproduce?
> >
>
> In fact, this is a probabilistic problem, we can only find this
> problem in large-scale product testing, and catch the coredump in a
> small number of Android mobile phones
>
> > Carl Eugen
Will apply， tks
diff mbox series

Patch

diff --git a/libavcodec/arm/hevcdsp_sao_neon.S b/libavcodec/arm/hevcdsp_sao_neon.S
index 3471679..8fd9d1e 100644
--- a/libavcodec/arm/hevcdsp_sao_neon.S
+++ b/libavcodec/arm/hevcdsp_sao_neon.S
@@ -35,10 +35,10 @@  function ff_hevc_sao_band_filter_neon_8, export=1
         vmov.u16    q15,  #1
         vmov.u8     q14,  #32
 0:      pld      [r1]
-        vld1.8   {d16},  [r1], r3
         cmp      r5,    #4
         beq      4f
 8:      subs     r4,    #1
+        vld1.8   {d16},  [r1], r3
         vshr.u8  d17,   d16,  #3   // index = [src>>3]
         vshll.u8 q9,    d17,  #1   // lowIndex = 2*index
         vadd.u16 q11,   q9,   q15  // highIndex = (2*index+1) << 8
@@ -54,7 +54,6 @@  function ff_hevc_sao_band_filter_neon_8, export=1
         vaddw.u8 q13,   q12,       d16
         vqmovun.s16      d8,         q13
         vst1.8    d8,   [r0],      r2
-        vld1.8   {d16}, [r1],      r3
         bne      8b
         subs     r5,    #8
         beq      99f
@@ -65,6 +64,7 @@  function ff_hevc_sao_band_filter_neon_8, export=1
         mov r1, r7
         b        0b
 4:      subs     r4,    #1
+        vld1.32   {d16[0]},  [r1],  r3
         vshr.u8  d17,   d16,  #3  // src>>3
         vshll.u8 q9,    d17,  #1   // lowIndex = 2*index
         vadd.u16 q11,   q9,   q15  // highIndex = (2*index+1) << 8
@@ -80,7 +80,6 @@  function ff_hevc_sao_band_filter_neon_8, export=1
         vaddw.u8 q13,   q12,       d16
         vqmovun.s16     d14,       q13
         vst1.32   d14[0],    [r0],     r2
-        vld1.32   {d16[0]},  [r1],     r3
         bne      4b
         b        99f
 99:
@@ -110,12 +109,12 @@  function ff_hevc_sao_edge_filter_neon_8, export=1
         mov      r11,    r1
         add      r11,    r9           // src[x + b_stride]
         pld      [r1]
-        vld1.8   {d16},  [r1],  r3    // src[x]  8x8bit
-        vld1.8   {d17},  [r10], r3    // src[x + a_stride]
-        vld1.8   {d18},  [r11], r3    // src[x + b_stride]
         cmp      r5,     #4
         beq      4f
 8:      subs     r4,     #1
+        vld1.8   {d16},  [r1],  r3    // src[x]  8x8bit
+        vld1.8   {d17},  [r10], r3    // src[x + a_stride]
+        vld1.8   {d18},  [r11], r3    // src[x + b_stride]
         vcgt.u8  d8,     d16,   d17
         vshr.u8  d9,     d8,    #7
         vclt.u8  d8,     d16,   d17
@@ -136,9 +135,6 @@  function ff_hevc_sao_edge_filter_neon_8, export=1
         vaddw.u8 q12,    q11,   d16
         vqmovun.s16      d26,   q12
         vst1.8   d26,    [r0],  r2
-        vld1.8   {d16},  [r1],  r3    // src[x]  8x8bit
-        vld1.8   {d17},  [r10], r3    // src[x + a_stride]
-        vld1.8   {d18},  [r11], r3    // src[x + b_stride]
         bne      8b
         subs     r5,     #8
         beq      99f
@@ -149,6 +145,9 @@  function ff_hevc_sao_edge_filter_neon_8, export=1
         mov      r1,     r7
         b        0b
 4:      subs     r4,    #1
+        vld1.32   {d16[0]},  [r1],  r3
+        vld1.32   {d17[0]},  [r10], r3    // src[x + a_stride]
+        vld1.32   {d18[0]},  [r11], r3    // src[x + b_stride]
         vcgt.u8  d8,     d16,   d17
         vshr.u8  d9,     d8,    #7
         vclt.u8  d8,     d16,   d17
@@ -169,9 +168,6 @@  function ff_hevc_sao_edge_filter_neon_8, export=1
         vaddw.u8 q12,    q11,   d16
         vqmovun.s16      d26,   q12
         vst1.32  d26[0], [r0],  r2
-        vld1.32   {d16[0]},  [r1],  r3
-        vld1.32   {d17[0]},  [r10], r3    // src[x + a_stride]
-        vld1.32   {d18[0]},  [r11], r3    // src[x + b_stride]
         bne      4b
         b        99f
 99: