Re: [FFmpeg-devel] [PATCH] avcodec/v210dec: Fix alignment check for AVX2

2019-05-27 Thread Michael Niedermayer
On Sat, May 18, 2019 at 10:32:42PM +0200, Michael Niedermayer wrote:
> On Sat, May 18, 2019 at 01:05:01PM +0200, Reimar Döffinger wrote:
> > On 18.05.2019, at 12:15, Michael Niedermayer  wrote:
> > 
> > > On Sat, May 18, 2019 at 12:02:55PM +0200, James Darnley wrote:
> > >> On 2019-05-18 09:39, Michael Niedermayer wrote:
> > >>> Fixes: "null pointer dereference"
> > >>> Fixes: 
> > >>> 14551/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_V210_fuzzer-5088609952071680
> > >>> 
> > >>> Found-by: continuous fuzzing process 
> > >>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > >>> Signed-off-by: Michael Niedermayer 
> > >>> ---
> > >>> libavcodec/v210dec.c | 2 +-
> > >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> > >>> 
> > >>> diff --git a/libavcodec/v210dec.c b/libavcodec/v210dec.c
> > >>> index bc1e1d34ff..5a33d8c089 100644
> > >>> --- a/libavcodec/v210dec.c
> > >>> +++ b/libavcodec/v210dec.c
> > >>> @@ -104,7 +104,7 @@ static int decode_frame(AVCodecContext *avctx, void 
> > >>> *data, int *got_frame,
> > >>> && avpkt->size - 64 >= stride * avctx->height)
> > >>> psrc += 64;
> > >>> 
> > >>> -aligned_input = !((uintptr_t)psrc & 0xf) && !(stride & 0xf);
> > >>> +aligned_input = !((uintptr_t)psrc & 0x1f) && !(stride & 0x1f);
> > >>> if (aligned_input != s->aligned_input) {
> > >>> s->aligned_input = aligned_input;
> > >>> ff_v210dec_init(s);
> > >>> 
> > >> 
> > >> Ah yes, that'll be needed after the recent addition of avx2.  LGTM and
> > >> sorry.
> > >> 
> > > 
> > >> I object to the commit message though because it isn't a "null pointer
> > >> dereference" but if that is the error as reported by the tool then keep
> > >> it as is.
> > > 
> > > yes, the tool(s) say things like "Null-dereference READ", "SEGV on 
> > > unknown address 0x"
> > 
> > Is that a tool bug or are we missing something that goes on here besides 
> > the alignment?
> > If it's a tool bug can we try to get it fixed?
> > This has a big risk of wasting time if they print straight-up wrong 
> > information...
> 
> I do not know for 100% sure. I actually have to admit that i tested on a 
> pre AVX2 CPU (sandybridge). I confirmed the bug and fix on a AVX2 box but
> the ossfuzz stuff is not setup on that box.
> What ossfuzz on the web interface says is that theres a null pointer 
> dereference in ff_v210_planar_unpack_aligned_avx2. And no anomaly is
> reproducable on sandybridge. 
> The tools seem to lack support for providing usefull information about asm
> code. Theres for example no line numbers, no register values, ...

> Ill try to report this issue in case this patch fixes it (and thus proofs
> that the alignment fix indeed matches the null pointer report.

reported:
https://github.com/google/oss-fuzz/issues/2465

[...]



-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] avcodec/v210dec: Fix alignment check for AVX2

2019-05-19 Thread Michael Niedermayer
On Sat, May 18, 2019 at 01:26:01PM +0200, James Darnley wrote:
> On 2019-05-18 12:15, Michael Niedermayer wrote:
> > On Sat, May 18, 2019 at 12:02:55PM +0200, James Darnley wrote:
> >> I object to the commit message though because it isn't a "null pointer
> >> dereference" but if that is the error as reported by the tool then keep
> >> it as is.
> > 
> > yes, the tool(s) say things like "Null-dereference READ", "SEGV on unknown 
> > address 0x"
> > 
> 
> Hm.  It is almost certainly an aligned move on an unaligned address.
> 
> I don't care that much about the rest of the commit message; the subject
> is correct which is good enough.

ok, will apply with a more clear reference to the null

thanks


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] avcodec/v210dec: Fix alignment check for AVX2

2019-05-18 Thread Michael Niedermayer
On Sat, May 18, 2019 at 01:05:01PM +0200, Reimar Döffinger wrote:
> On 18.05.2019, at 12:15, Michael Niedermayer  wrote:
> 
> > On Sat, May 18, 2019 at 12:02:55PM +0200, James Darnley wrote:
> >> On 2019-05-18 09:39, Michael Niedermayer wrote:
> >>> Fixes: "null pointer dereference"
> >>> Fixes: 
> >>> 14551/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_V210_fuzzer-5088609952071680
> >>> 
> >>> Found-by: continuous fuzzing process 
> >>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> >>> Signed-off-by: Michael Niedermayer 
> >>> ---
> >>> libavcodec/v210dec.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>> 
> >>> diff --git a/libavcodec/v210dec.c b/libavcodec/v210dec.c
> >>> index bc1e1d34ff..5a33d8c089 100644
> >>> --- a/libavcodec/v210dec.c
> >>> +++ b/libavcodec/v210dec.c
> >>> @@ -104,7 +104,7 @@ static int decode_frame(AVCodecContext *avctx, void 
> >>> *data, int *got_frame,
> >>> && avpkt->size - 64 >= stride * avctx->height)
> >>> psrc += 64;
> >>> 
> >>> -aligned_input = !((uintptr_t)psrc & 0xf) && !(stride & 0xf);
> >>> +aligned_input = !((uintptr_t)psrc & 0x1f) && !(stride & 0x1f);
> >>> if (aligned_input != s->aligned_input) {
> >>> s->aligned_input = aligned_input;
> >>> ff_v210dec_init(s);
> >>> 
> >> 
> >> Ah yes, that'll be needed after the recent addition of avx2.  LGTM and
> >> sorry.
> >> 
> > 
> >> I object to the commit message though because it isn't a "null pointer
> >> dereference" but if that is the error as reported by the tool then keep
> >> it as is.
> > 
> > yes, the tool(s) say things like "Null-dereference READ", "SEGV on unknown 
> > address 0x"
> 
> Is that a tool bug or are we missing something that goes on here besides the 
> alignment?
> If it's a tool bug can we try to get it fixed?
> This has a big risk of wasting time if they print straight-up wrong 
> information...

I do not know for 100% sure. I actually have to admit that i tested on a 
pre AVX2 CPU (sandybridge). I confirmed the bug and fix on a AVX2 box but
the ossfuzz stuff is not setup on that box.
What ossfuzz on the web interface says is that theres a null pointer 
dereference in ff_v210_planar_unpack_aligned_avx2. And no anomaly is
reproducable on sandybridge. 
The tools seem to lack support for providing usefull information about asm
code. Theres for example no line numbers, no register values, ...
Ill try to report this issue in case this patch fixes it (and thus proofs
that the alignment fix indeed matches the null pointer report.

Thanks

PS: yes i know i should upgrade that sandybridge box, i had planed to but
theres always something that causes me to move it down on my todo list ...


-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] avcodec/v210dec: Fix alignment check for AVX2

2019-05-18 Thread James Darnley
On 2019-05-18 12:15, Michael Niedermayer wrote:
> On Sat, May 18, 2019 at 12:02:55PM +0200, James Darnley wrote:
>> I object to the commit message though because it isn't a "null pointer
>> dereference" but if that is the error as reported by the tool then keep
>> it as is.
> 
> yes, the tool(s) say things like "Null-dereference READ", "SEGV on unknown 
> address 0x"
> 

Hm.  It is almost certainly an aligned move on an unaligned address.

I don't care that much about the rest of the commit message; the subject
is correct which is good enough.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] avcodec/v210dec: Fix alignment check for AVX2

2019-05-18 Thread Reimar Döffinger
On 18.05.2019, at 12:15, Michael Niedermayer  wrote:

> On Sat, May 18, 2019 at 12:02:55PM +0200, James Darnley wrote:
>> On 2019-05-18 09:39, Michael Niedermayer wrote:
>>> Fixes: "null pointer dereference"
>>> Fixes: 
>>> 14551/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_V210_fuzzer-5088609952071680
>>> 
>>> Found-by: continuous fuzzing process 
>>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by: Michael Niedermayer 
>>> ---
>>> libavcodec/v210dec.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/libavcodec/v210dec.c b/libavcodec/v210dec.c
>>> index bc1e1d34ff..5a33d8c089 100644
>>> --- a/libavcodec/v210dec.c
>>> +++ b/libavcodec/v210dec.c
>>> @@ -104,7 +104,7 @@ static int decode_frame(AVCodecContext *avctx, void 
>>> *data, int *got_frame,
>>> && avpkt->size - 64 >= stride * avctx->height)
>>> psrc += 64;
>>> 
>>> -aligned_input = !((uintptr_t)psrc & 0xf) && !(stride & 0xf);
>>> +aligned_input = !((uintptr_t)psrc & 0x1f) && !(stride & 0x1f);
>>> if (aligned_input != s->aligned_input) {
>>> s->aligned_input = aligned_input;
>>> ff_v210dec_init(s);
>>> 
>> 
>> Ah yes, that'll be needed after the recent addition of avx2.  LGTM and
>> sorry.
>> 
> 
>> I object to the commit message though because it isn't a "null pointer
>> dereference" but if that is the error as reported by the tool then keep
>> it as is.
> 
> yes, the tool(s) say things like "Null-dereference READ", "SEGV on unknown 
> address 0x"

Is that a tool bug or are we missing something that goes on here besides the 
alignment?
If it's a tool bug can we try to get it fixed?
This has a big risk of wasting time if they print straight-up wrong 
information...
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] avcodec/v210dec: Fix alignment check for AVX2

2019-05-18 Thread Michael Niedermayer
On Sat, May 18, 2019 at 12:02:55PM +0200, James Darnley wrote:
> On 2019-05-18 09:39, Michael Niedermayer wrote:
> > Fixes: "null pointer dereference"
> > Fixes: 
> > 14551/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_V210_fuzzer-5088609952071680
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/v210dec.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/v210dec.c b/libavcodec/v210dec.c
> > index bc1e1d34ff..5a33d8c089 100644
> > --- a/libavcodec/v210dec.c
> > +++ b/libavcodec/v210dec.c
> > @@ -104,7 +104,7 @@ static int decode_frame(AVCodecContext *avctx, void 
> > *data, int *got_frame,
> >  && avpkt->size - 64 >= stride * avctx->height)
> >  psrc += 64;
> >  
> > -aligned_input = !((uintptr_t)psrc & 0xf) && !(stride & 0xf);
> > +aligned_input = !((uintptr_t)psrc & 0x1f) && !(stride & 0x1f);
> >  if (aligned_input != s->aligned_input) {
> >  s->aligned_input = aligned_input;
> >  ff_v210dec_init(s);
> > 
> 
> Ah yes, that'll be needed after the recent addition of avx2.  LGTM and
> sorry.
> 

> I object to the commit message though because it isn't a "null pointer
> dereference" but if that is the error as reported by the tool then keep
> it as is.

yes, the tool(s) say things like "Null-dereference READ", "SEGV on unknown 
address 0x"

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] avcodec/v210dec: Fix alignment check for AVX2

2019-05-18 Thread James Darnley
On 2019-05-18 09:39, Michael Niedermayer wrote:
> Fixes: "null pointer dereference"
> Fixes: 
> 14551/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_V210_fuzzer-5088609952071680
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/v210dec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/v210dec.c b/libavcodec/v210dec.c
> index bc1e1d34ff..5a33d8c089 100644
> --- a/libavcodec/v210dec.c
> +++ b/libavcodec/v210dec.c
> @@ -104,7 +104,7 @@ static int decode_frame(AVCodecContext *avctx, void 
> *data, int *got_frame,
>  && avpkt->size - 64 >= stride * avctx->height)
>  psrc += 64;
>  
> -aligned_input = !((uintptr_t)psrc & 0xf) && !(stride & 0xf);
> +aligned_input = !((uintptr_t)psrc & 0x1f) && !(stride & 0x1f);
>  if (aligned_input != s->aligned_input) {
>  s->aligned_input = aligned_input;
>  ff_v210dec_init(s);
> 

Ah yes, that'll be needed after the recent addition of avx2.  LGTM and
sorry.

I object to the commit message though because it isn't a "null pointer
dereference" but if that is the error as reported by the tool then keep
it as is.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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