Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] avcodec/decode: guard against NULL hw_frames_ctx
@ 2023-11-14 18:22 Dmitry Rogozhkin
  2023-11-14 18:32 ` Hendrik Leppkes
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Dmitry Rogozhkin @ 2023-11-14 18:22 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Christoph Reiter, Dmitry Rogozhkin, Lynne

Gurd against segfault running VLC decoding under msys2 [1]:

Thread 33 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 37728.0xadd0]
ff_hwaccel_frame_priv_alloc (avctx=0x6447b00, hwaccel_picture_private=0x65dfd00)
    at libavcodec/decode.c:1848
1848        frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
(gdb) bt
    at libavcodec/decode.c:1848
    at libavcodec/h264_slice.c:208
    first_slice=1) at libavcodec/h264_slice.c:1599
    at libavcodec/h264_slice.c:2130
    at libavcodec/h264dec.c:652
    got_frame=0x646e4b0, avpkt=0x64522c0) at libavcodec/h264dec.c:1048

(gdb) p avctx
$1 = (AVCodecContext *) 0x6447b00
(gdb) p avctx->hw_frames_ctx
$2 = (AVBufferRef *) 0x0

See[1]: https://github.com/msys2/MINGW-packages/pull/19050
Fixes: be07145109 ("avcodec: add AVHWAccel.free_frame_priv callback")
CC: Lynne <dev@lynne.ee>
CC: Christoph Reiter <reiter.christoph@gmail.com>
Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
---
 libavcodec/decode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index ad39021..3caaeec 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1845,9 +1845,9 @@ int ff_hwaccel_frame_priv_alloc(AVCodecContext *avctx, void **hwaccel_picture_pr
 
     av_assert0(!*hwaccel_picture_private);
 
-    frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
+    frames_ctx = (AVHWFramesContext *) avctx->hw_frames_ctx ? avctx->hw_frames_ctx->data : NULL;
     *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel->frame_priv_data_size, 0,
-                                                      frames_ctx->device_ctx,
+                                                      frames_ctx ? frames_ctx->device_ctx : NULL,
                                                       hwaccel->free_frame_priv);
     if (!*hwaccel_picture_private)
         return AVERROR(ENOMEM);
-- 
1.8.3.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".

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

* Re: [FFmpeg-devel] [PATCH] avcodec/decode: guard against NULL hw_frames_ctx
  2023-11-14 18:22 [FFmpeg-devel] [PATCH] avcodec/decode: guard against NULL hw_frames_ctx Dmitry Rogozhkin
@ 2023-11-14 18:32 ` Hendrik Leppkes
  2023-11-15 15:28   ` Rogozhkin, Dmitry V
  2023-11-14 19:14 ` [FFmpeg-devel] [PATCH v2] " Dmitry Rogozhkin
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Hendrik Leppkes @ 2023-11-14 18:32 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Tue, Nov 14, 2023 at 7:23 PM Dmitry Rogozhkin
<dmitry.v.rogozhkin-at-intel.com@ffmpeg.org> wrote:
>
> Gurd against segfault running VLC decoding under msys2 [1]:
>
> Thread 33 received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 37728.0xadd0]
> ff_hwaccel_frame_priv_alloc (avctx=0x6447b00, hwaccel_picture_private=0x65dfd00)
>     at libavcodec/decode.c:1848
> 1848        frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
> (gdb) bt
>     at libavcodec/decode.c:1848
>     at libavcodec/h264_slice.c:208
>     first_slice=1) at libavcodec/h264_slice.c:1599
>     at libavcodec/h264_slice.c:2130
>     at libavcodec/h264dec.c:652
>     got_frame=0x646e4b0, avpkt=0x64522c0) at libavcodec/h264dec.c:1048
>
> (gdb) p avctx
> $1 = (AVCodecContext *) 0x6447b00
> (gdb) p avctx->hw_frames_ctx
> $2 = (AVBufferRef *) 0x0
>
> See[1]: https://github.com/msys2/MINGW-packages/pull/19050
> Fixes: be07145109 ("avcodec: add AVHWAccel.free_frame_priv callback")
> CC: Lynne <dev@lynne.ee>
> CC: Christoph Reiter <reiter.christoph@gmail.com>
> Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> ---
>  libavcodec/decode.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index ad39021..3caaeec 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1845,9 +1845,9 @@ int ff_hwaccel_frame_priv_alloc(AVCodecContext *avctx, void **hwaccel_picture_pr
>
>      av_assert0(!*hwaccel_picture_private);
>
> -    frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
> +    frames_ctx = (AVHWFramesContext *) avctx->hw_frames_ctx ? avctx->hw_frames_ctx->data : NULL;
>      *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel->frame_priv_data_size, 0,
> -                                                      frames_ctx->device_ctx,
> +                                                      frames_ctx ? frames_ctx->device_ctx : NULL,
>                                                        hwaccel->free_frame_priv);

The free_frame_priv callback is not usable when no context is
available. The code should be updated to check if the hwaccel has a
free_frame_priv callback and then require a frames_ctx to be set and
otherwise error out (instead of crashing), and if free_frame_priv is
not set, then it can allocate a buffer without these requirements.

- Hendrik
_______________________________________________
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] 14+ messages in thread

* [FFmpeg-devel] [PATCH v2] avcodec/decode: guard against NULL hw_frames_ctx
  2023-11-14 18:22 [FFmpeg-devel] [PATCH] avcodec/decode: guard against NULL hw_frames_ctx Dmitry Rogozhkin
  2023-11-14 18:32 ` Hendrik Leppkes
@ 2023-11-14 19:14 ` Dmitry Rogozhkin
  2023-11-17 15:41   ` Rogozhkin, Dmitry V
  2023-11-17 17:03 ` [FFmpeg-devel] [PATCH v3] " Dmitry Rogozhkin
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Dmitry Rogozhkin @ 2023-11-14 19:14 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Christoph Reiter, Dmitry Rogozhkin, Lynne

Guard against segfault running VLC decoding under msys2 [1]:

Thread 33 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 37728.0xadd0]
ff_hwaccel_frame_priv_alloc (avctx=0x6447b00, hwaccel_picture_private=0x65dfd00)
    at libavcodec/decode.c:1848
1848        frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
(gdb) bt
    at libavcodec/decode.c:1848
    at libavcodec/h264_slice.c:208
    first_slice=1) at libavcodec/h264_slice.c:1599
    at libavcodec/h264_slice.c:2130
    at libavcodec/h264dec.c:652
    got_frame=0x646e4b0, avpkt=0x64522c0) at libavcodec/h264dec.c:1048

(gdb) p avctx
$1 = (AVCodecContext *) 0x6447b00
(gdb) p avctx->hw_frames_ctx
$2 = (AVBufferRef *) 0x0

v2: check for free_frame_priv (Hendrik)

See[1]: https://github.com/msys2/MINGW-packages/pull/19050
Fixes: be07145109 ("avcodec: add AVHWAccel.free_frame_priv callback")
CC: Lynne <dev@lynne.ee>
CC: Christoph Reiter <reiter.christoph@gmail.com>
Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
---
 libavcodec/decode.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index ad39021..58f887d 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1838,17 +1838,29 @@ int ff_copy_palette(void *dst, const AVPacket *src, void *logctx)
 int ff_hwaccel_frame_priv_alloc(AVCodecContext *avctx, void **hwaccel_picture_private)
 {
     const FFHWAccel *hwaccel = ffhwaccel(avctx->hwaccel);
-    AVHWFramesContext *frames_ctx;
 
     if (!hwaccel || !hwaccel->frame_priv_data_size)
         return 0;
 
     av_assert0(!*hwaccel_picture_private);
 
-    frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
-    *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel->frame_priv_data_size, 0,
-                                                      frames_ctx->device_ctx,
-                                                      hwaccel->free_frame_priv);
+    if (hwaccel->free_frame_priv) {
+        AVHWFramesContext *frames_ctx;
+
+        if (!avctx->hw_frames_ctx)
+            return AVERROR(ENOMEM);
+
+        frames_ctx = (AVHWFramesContext *) avctx->hw_frames_ctx->data;
+        if (!frames_ctx)
+            return AVERROR(ENOMEM);
+
+        *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel->frame_priv_data_size, 0,
+                                                          frames_ctx->device_ctx,
+                                                          hwaccel->free_frame_priv);
+    } else {
+        *hwaccel_picture_private = ff_refstruct_allocz(hwaccel->frame_priv_data_size);
+    }
+
     if (!*hwaccel_picture_private)
         return AVERROR(ENOMEM);
 
-- 
1.8.3.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".

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

* Re: [FFmpeg-devel] [PATCH] avcodec/decode: guard against NULL hw_frames_ctx
  2023-11-14 18:32 ` Hendrik Leppkes
@ 2023-11-15 15:28   ` Rogozhkin, Dmitry V
  0 siblings, 0 replies; 14+ messages in thread
From: Rogozhkin, Dmitry V @ 2023-11-15 15:28 UTC (permalink / raw)
  To: ffmpeg-devel

On Tue, 2023-11-14 at 19:32 +0100, Hendrik Leppkes wrote:
> On Tue, Nov 14, 2023 at 7:23 PM Dmitry Rogozhkin
> <dmitry.v.rogozhkin-at-intel.com@ffmpeg.org> wrote:
> > 
> > Gurd against segfault running VLC decoding under msys2 [1]:
> > 
> > Thread 33 received signal SIGSEGV, Segmentation fault.
> > [Switching to Thread 37728.0xadd0]
> > ff_hwaccel_frame_priv_alloc (avctx=0x6447b00,
> > hwaccel_picture_private=0x65dfd00)
> >     at libavcodec/decode.c:1848
> > 1848        frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx-
> > >data;
> > (gdb) bt
> >     at libavcodec/decode.c:1848
> >     at libavcodec/h264_slice.c:208
> >     first_slice=1) at libavcodec/h264_slice.c:1599
> >     at libavcodec/h264_slice.c:2130
> >     at libavcodec/h264dec.c:652
> >     got_frame=0x646e4b0, avpkt=0x64522c0) at
> > libavcodec/h264dec.c:1048
> > 
> > (gdb) p avctx
> > $1 = (AVCodecContext *) 0x6447b00
> > (gdb) p avctx->hw_frames_ctx
> > $2 = (AVBufferRef *) 0x0
> > 
> > See[1]: https://github.com/msys2/MINGW-packages/pull/19050
> > Fixes: be07145109 ("avcodec: add AVHWAccel.free_frame_priv
> > callback")
> > CC: Lynne <dev@lynne.ee>
> > CC: Christoph Reiter <reiter.christoph@gmail.com>
> > Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> > ---
> >  libavcodec/decode.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> > index ad39021..3caaeec 100644
> > --- a/libavcodec/decode.c
> > +++ b/libavcodec/decode.c
> > @@ -1845,9 +1845,9 @@ int
> > ff_hwaccel_frame_priv_alloc(AVCodecContext *avctx, void
> > **hwaccel_picture_pr
> > 
> >      av_assert0(!*hwaccel_picture_private);
> > 
> > -    frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
> > +    frames_ctx = (AVHWFramesContext *) avctx->hw_frames_ctx ?
> > avctx->hw_frames_ctx->data : NULL;
> >      *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel-
> > >frame_priv_data_size, 0,
> > -                                                      frames_ctx-
> > >device_ctx,
> > +                                                      frames_ctx ?
> > frames_ctx->device_ctx : NULL,
> >                                                        hwaccel-
> > >free_frame_priv);
> 
> The free_frame_priv callback is not usable when no context is
> available. The code should be updated to check if the hwaccel has a
> free_frame_priv callback and then require a frames_ctx to be set and
> otherwise error out (instead of crashing), and if free_frame_priv is
> not set, then it can allocate a buffer without these requirements.

Thank you for feedback. I hope I have fixed this and sent v2 update
yesterday. Can you, please, help review again?

> 
> - Hendrik
> _______________________________________________
> 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".
_______________________________________________
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] 14+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] avcodec/decode: guard against NULL hw_frames_ctx
  2023-11-14 19:14 ` [FFmpeg-devel] [PATCH v2] " Dmitry Rogozhkin
@ 2023-11-17 15:41   ` Rogozhkin, Dmitry V
  0 siblings, 0 replies; 14+ messages in thread
From: Rogozhkin, Dmitry V @ 2023-11-17 15:41 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: reiter.christoph, dev

On Tue, 2023-11-14 at 11:14 -0800, Dmitry Rogozhkin wrote:
> Guard against segfault running VLC decoding under msys2 [1]:
> 
> Thread 33 received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 37728.0xadd0]
> ff_hwaccel_frame_priv_alloc (avctx=0x6447b00,
> hwaccel_picture_private=0x65dfd00)
>     at libavcodec/decode.c:1848
> 1848        frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx-
> >data;
> (gdb) bt
>     at libavcodec/decode.c:1848
>     at libavcodec/h264_slice.c:208
>     first_slice=1) at libavcodec/h264_slice.c:1599
>     at libavcodec/h264_slice.c:2130
>     at libavcodec/h264dec.c:652
>     got_frame=0x646e4b0, avpkt=0x64522c0) at
> libavcodec/h264dec.c:1048
> 
> (gdb) p avctx
> $1 = (AVCodecContext *) 0x6447b00
> (gdb) p avctx->hw_frames_ctx
> $2 = (AVBufferRef *) 0x0
> 
> v2: check for free_frame_priv (Hendrik)
> 
> See[1]: https://github.com/msys2/MINGW-packages/pull/19050

Posting below some comments done in this PR for this patch.

> Fixes: be07145109 ("avcodec: add AVHWAccel.free_frame_priv callback")
> CC: Lynne <dev@lynne.ee>
> CC: Christoph Reiter <reiter.christoph@gmail.com>
> Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> ---
>  libavcodec/decode.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index ad39021..58f887d 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1838,17 +1838,29 @@ int ff_copy_palette(void *dst, const AVPacket
> *src, void *logctx)
>  int ff_hwaccel_frame_priv_alloc(AVCodecContext *avctx, void
> **hwaccel_picture_private)
>  {
>      const FFHWAccel *hwaccel = ffhwaccel(avctx->hwaccel);
> -    AVHWFramesContext *frames_ctx;
>  
>      if (!hwaccel || !hwaccel->frame_priv_data_size)
>          return 0;
>  
>      av_assert0(!*hwaccel_picture_private);
>  
> -    frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
> -    *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel-
> >frame_priv_data_size, 0,
> -                                                      frames_ctx-
> >device_ctx,
> -                                                      hwaccel-
> >free_frame_priv);
> +    if (hwaccel->free_frame_priv) {
> +        AVHWFramesContext *frames_ctx;
> +
> +        if (!avctx->hw_frames_ctx)
> +            return AVERROR(ENOMEM);

Similar other places log and return EINVAL:
https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/decode.c#L1121-L1123
ENOMEM seems like the wrong status here since there is no allocation
happening.

> +
> +        frames_ctx = (AVHWFramesContext *) avctx->hw_frames_ctx-
> >data;
> +        if (!frames_ctx)
> +            return AVERROR(ENOMEM);

Can hw_frames_ctx->data really be null here? i.e. is this check needed?
If yes, then ENOMEM looks wrong here too.

> +
> +        *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel-
> >frame_priv_data_size, 0,
> +                                                          frames_ctx
> ->device_ctx,
> +                                                          hwaccel-
> >free_frame_priv);
> +    } else {
> +        *hwaccel_picture_private = ff_refstruct_allocz(hwaccel-
> >frame_priv_data_size);
> +    }
> +
>      if (!*hwaccel_picture_private)
>          return AVERROR(ENOMEM);
>  
_______________________________________________
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] 14+ messages in thread

* [FFmpeg-devel] [PATCH v3] avcodec/decode: guard against NULL hw_frames_ctx
  2023-11-14 18:22 [FFmpeg-devel] [PATCH] avcodec/decode: guard against NULL hw_frames_ctx Dmitry Rogozhkin
  2023-11-14 18:32 ` Hendrik Leppkes
  2023-11-14 19:14 ` [FFmpeg-devel] [PATCH v2] " Dmitry Rogozhkin
@ 2023-11-17 17:03 ` Dmitry Rogozhkin
  2023-11-19 10:10   ` Sebastian Ramacher
  2023-11-19 15:29   ` Hendrik Leppkes
  2023-11-21  4:37 ` [FFmpeg-devel] [PATCH v4] avcodec/decode: validate hw_frames_ctx when AVHWAccel.free_frame_priv is used Dmitry Rogozhkin
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 14+ messages in thread
From: Dmitry Rogozhkin @ 2023-11-17 17:03 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Christoph Reiter, Dmitry Rogozhkin, Lynne

Guard against segfault running VLC decoding under msys2 [1]:

Thread 33 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 37728.0xadd0]
ff_hwaccel_frame_priv_alloc (avctx=0x6447b00, hwaccel_picture_private=0x65dfd00)
    at libavcodec/decode.c:1848
1848        frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
(gdb) bt
    at libavcodec/decode.c:1848
    at libavcodec/h264_slice.c:208
    first_slice=1) at libavcodec/h264_slice.c:1599
    at libavcodec/h264_slice.c:2130
    at libavcodec/h264dec.c:652
    got_frame=0x646e4b0, avpkt=0x64522c0) at libavcodec/h264dec.c:1048

(gdb) p avctx
$1 = (AVCodecContext *) 0x6447b00
(gdb) p avctx->hw_frames_ctx
$2 = (AVBufferRef *) 0x0

v2: check for free_frame_priv (Hendrik)
v3: return EINVAL (Christoph Reiter)

See[1]: https://github.com/msys2/MINGW-packages/pull/19050
Fixes: be07145109 ("avcodec: add AVHWAccel.free_frame_priv callback")
CC: Lynne <dev@lynne.ee>
CC: Christoph Reiter <reiter.christoph@gmail.com>
Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
---
 libavcodec/decode.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index ad39021..50c3995 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1838,17 +1838,25 @@ int ff_copy_palette(void *dst, const AVPacket *src, void *logctx)
 int ff_hwaccel_frame_priv_alloc(AVCodecContext *avctx, void **hwaccel_picture_private)
 {
     const FFHWAccel *hwaccel = ffhwaccel(avctx->hwaccel);
-    AVHWFramesContext *frames_ctx;
 
     if (!hwaccel || !hwaccel->frame_priv_data_size)
         return 0;
 
     av_assert0(!*hwaccel_picture_private);
 
-    frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
-    *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel->frame_priv_data_size, 0,
-                                                      frames_ctx->device_ctx,
-                                                      hwaccel->free_frame_priv);
+    if (hwaccel->free_frame_priv) {
+        AVHWFramesContext *frames_ctx;
+
+        if (!avctx->hw_frames_ctx)
+            return AVERROR(EINVAL);
+
+        *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel->frame_priv_data_size, 0,
+                                                          frames_ctx->device_ctx,
+                                                          hwaccel->free_frame_priv);
+    } else {
+        *hwaccel_picture_private = ff_refstruct_allocz(hwaccel->frame_priv_data_size);
+    }
+
     if (!*hwaccel_picture_private)
         return AVERROR(ENOMEM);
 
-- 
1.8.3.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".

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

* Re: [FFmpeg-devel] [PATCH v3] avcodec/decode: guard against NULL hw_frames_ctx
  2023-11-17 17:03 ` [FFmpeg-devel] [PATCH v3] " Dmitry Rogozhkin
@ 2023-11-19 10:10   ` Sebastian Ramacher
  2023-11-19 15:29   ` Hendrik Leppkes
  1 sibling, 0 replies; 14+ messages in thread
From: Sebastian Ramacher @ 2023-11-19 10:10 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On 2023-11-17 09:03:03 -0800, Dmitry Rogozhkin wrote:
> Guard against segfault running VLC decoding under msys2 [1]:
> 
> Thread 33 received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 37728.0xadd0]
> ff_hwaccel_frame_priv_alloc (avctx=0x6447b00, hwaccel_picture_private=0x65dfd00)
>     at libavcodec/decode.c:1848
> 1848        frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
> (gdb) bt
>     at libavcodec/decode.c:1848
>     at libavcodec/h264_slice.c:208
>     first_slice=1) at libavcodec/h264_slice.c:1599
>     at libavcodec/h264_slice.c:2130
>     at libavcodec/h264dec.c:652
>     got_frame=0x646e4b0, avpkt=0x64522c0) at libavcodec/h264dec.c:1048
> 
> (gdb) p avctx
> $1 = (AVCodecContext *) 0x6447b00
> (gdb) p avctx->hw_frames_ctx
> $2 = (AVBufferRef *) 0x0
> 
> v2: check for free_frame_priv (Hendrik)
> v3: return EINVAL (Christoph Reiter)
> 
> See[1]: https://github.com/msys2/MINGW-packages/pull/19050
> Fixes: be07145109 ("avcodec: add AVHWAccel.free_frame_priv callback")
> CC: Lynne <dev@lynne.ee>
> CC: Christoph Reiter <reiter.christoph@gmail.com>
> Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>

We have also seen the same issue on Debian unstable with VDPAU playback
in vlc. See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1055952. I
can confirm that this patch fixes the issue. Please apply the patch and
backport it to 6.1.

Cheers

> ---
>  libavcodec/decode.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index ad39021..50c3995 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1838,17 +1838,25 @@ int ff_copy_palette(void *dst, const AVPacket *src, void *logctx)
>  int ff_hwaccel_frame_priv_alloc(AVCodecContext *avctx, void **hwaccel_picture_private)
>  {
>      const FFHWAccel *hwaccel = ffhwaccel(avctx->hwaccel);
> -    AVHWFramesContext *frames_ctx;
>  
>      if (!hwaccel || !hwaccel->frame_priv_data_size)
>          return 0;
>  
>      av_assert0(!*hwaccel_picture_private);
>  
> -    frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
> -    *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel->frame_priv_data_size, 0,
> -                                                      frames_ctx->device_ctx,
> -                                                      hwaccel->free_frame_priv);
> +    if (hwaccel->free_frame_priv) {
> +        AVHWFramesContext *frames_ctx;
> +
> +        if (!avctx->hw_frames_ctx)
> +            return AVERROR(EINVAL);
> +
> +        *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel->frame_priv_data_size, 0,
> +                                                          frames_ctx->device_ctx,
> +                                                          hwaccel->free_frame_priv);
> +    } else {
> +        *hwaccel_picture_private = ff_refstruct_allocz(hwaccel->frame_priv_data_size);
> +    }
> +
>      if (!*hwaccel_picture_private)
>          return AVERROR(ENOMEM);
>  
> -- 
> 1.8.3.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".
_______________________________________________
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] 14+ messages in thread

* Re: [FFmpeg-devel] [PATCH v3] avcodec/decode: guard against NULL hw_frames_ctx
  2023-11-17 17:03 ` [FFmpeg-devel] [PATCH v3] " Dmitry Rogozhkin
  2023-11-19 10:10   ` Sebastian Ramacher
@ 2023-11-19 15:29   ` Hendrik Leppkes
  2023-11-21  4:45     ` Rogozhkin, Dmitry V
  1 sibling, 1 reply; 14+ messages in thread
From: Hendrik Leppkes @ 2023-11-19 15:29 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Fri, Nov 17, 2023 at 6:04 PM Dmitry Rogozhkin
<dmitry.v.rogozhkin-at-intel.com@ffmpeg.org> wrote:
>
> Guard against segfault running VLC decoding under msys2 [1]:
>
> Thread 33 received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 37728.0xadd0]
> ff_hwaccel_frame_priv_alloc (avctx=0x6447b00, hwaccel_picture_private=0x65dfd00)
>     at libavcodec/decode.c:1848
> 1848        frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
> (gdb) bt
>     at libavcodec/decode.c:1848
>     at libavcodec/h264_slice.c:208
>     first_slice=1) at libavcodec/h264_slice.c:1599
>     at libavcodec/h264_slice.c:2130
>     at libavcodec/h264dec.c:652
>     got_frame=0x646e4b0, avpkt=0x64522c0) at libavcodec/h264dec.c:1048
>
> (gdb) p avctx
> $1 = (AVCodecContext *) 0x6447b00
> (gdb) p avctx->hw_frames_ctx
> $2 = (AVBufferRef *) 0x0
>
> v2: check for free_frame_priv (Hendrik)
> v3: return EINVAL (Christoph Reiter)
>
> See[1]: https://github.com/msys2/MINGW-packages/pull/19050
> Fixes: be07145109 ("avcodec: add AVHWAccel.free_frame_priv callback")
> CC: Lynne <dev@lynne.ee>
> CC: Christoph Reiter <reiter.christoph@gmail.com>
> Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> ---

The latest change itself looks fine to me, I would however prefer if
the commit message would contain a bit more text and a bit less gdb
dump.

Maybe something like this (quick suggestion, for title and body):

avcodec: validate hw_frames_ctx is available when
AVHWAccel.free_frame_priv is used

Validate that a hw_frames_ctx is available before using it for the
AVHWAccel.free_frame_priv callback, and don't require it to be present when
the callback is not in use by the HWAccel.

---

Feel free to augment and reword, but I don't think the gdb dump offers
much info that couldn't be conveyed more clearly in a few words that
mention the absence of hw_frame_ctx.

- Hendrik
_______________________________________________
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] 14+ messages in thread

* [FFmpeg-devel] [PATCH v4] avcodec/decode: validate hw_frames_ctx when AVHWAccel.free_frame_priv is used
  2023-11-14 18:22 [FFmpeg-devel] [PATCH] avcodec/decode: guard against NULL hw_frames_ctx Dmitry Rogozhkin
                   ` (2 preceding siblings ...)
  2023-11-17 17:03 ` [FFmpeg-devel] [PATCH v3] " Dmitry Rogozhkin
@ 2023-11-21  4:37 ` Dmitry Rogozhkin
       [not found] ` <1700541472-12254-1-git-send-email-dmitry.v.rogozhkin@intel.com-NjkA62m----9>
  2023-11-21  5:57 ` [FFmpeg-devel] [PATCH v5] " Dmitry Rogozhkin
  5 siblings, 0 replies; 14+ messages in thread
From: Dmitry Rogozhkin @ 2023-11-21  4:37 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Christoph Reiter, Dmitry Rogozhkin, Lynne

Validate that a hw_frames_ctx is available before using it for
the AVHWAccel.free_frame_priv callback, and don't require it to
be present when the callback is not in use by the HWAccel.

v2: check for free_frame_priv (Hendrik)
v3: return EINVAL (Christoph Reiter)
v4: better commit message (Hendrik)

See[1]: https://github.com/msys2/MINGW-packages/pull/19050
Fixes: be07145109 ("avcodec: add AVHWAccel.free_frame_priv callback")
CC: Lynne <dev@lynne.ee>
CC: Christoph Reiter <reiter.christoph@gmail.com>
Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
---
 libavcodec/decode.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index ad39021..50c3995 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1838,17 +1838,25 @@ int ff_copy_palette(void *dst, const AVPacket *src, void *logctx)
 int ff_hwaccel_frame_priv_alloc(AVCodecContext *avctx, void **hwaccel_picture_private)
 {
     const FFHWAccel *hwaccel = ffhwaccel(avctx->hwaccel);
-    AVHWFramesContext *frames_ctx;
 
     if (!hwaccel || !hwaccel->frame_priv_data_size)
         return 0;
 
     av_assert0(!*hwaccel_picture_private);
 
-    frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
-    *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel->frame_priv_data_size, 0,
-                                                      frames_ctx->device_ctx,
-                                                      hwaccel->free_frame_priv);
+    if (hwaccel->free_frame_priv) {
+        AVHWFramesContext *frames_ctx;
+
+        if (!avctx->hw_frames_ctx)
+            return AVERROR(EINVAL);
+
+        *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel->frame_priv_data_size, 0,
+                                                          frames_ctx->device_ctx,
+                                                          hwaccel->free_frame_priv);
+    } else {
+        *hwaccel_picture_private = ff_refstruct_allocz(hwaccel->frame_priv_data_size);
+    }
+
     if (!*hwaccel_picture_private)
         return AVERROR(ENOMEM);
 
-- 
1.8.3.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".

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

* Re: [FFmpeg-devel] [PATCH v3] avcodec/decode: guard against NULL hw_frames_ctx
  2023-11-19 15:29   ` Hendrik Leppkes
@ 2023-11-21  4:45     ` Rogozhkin, Dmitry V
  0 siblings, 0 replies; 14+ messages in thread
From: Rogozhkin, Dmitry V @ 2023-11-21  4:45 UTC (permalink / raw)
  To: ffmpeg-devel

On Sun, 2023-11-19 at 16:29 +0100, Hendrik Leppkes wrote:
> On Fri, Nov 17, 2023 at 6:04 PM Dmitry Rogozhkin
> <dmitry.v.rogozhkin-at-intel.com@ffmpeg.org> wrote:
> > 
> > Guard against segfault running VLC decoding under msys2 [1]:
> > 
> > Thread 33 received signal SIGSEGV, Segmentation fault.
> > [Switching to Thread 37728.0xadd0]
> > ff_hwaccel_frame_priv_alloc (avctx=0x6447b00,
> > hwaccel_picture_private=0x65dfd00)
> >     at libavcodec/decode.c:1848
> > 1848        frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx-
> > >data;
> > (gdb) bt
> >     at libavcodec/decode.c:1848
> >     at libavcodec/h264_slice.c:208
> >     first_slice=1) at libavcodec/h264_slice.c:1599
> >     at libavcodec/h264_slice.c:2130
> >     at libavcodec/h264dec.c:652
> >     got_frame=0x646e4b0, avpkt=0x64522c0) at
> > libavcodec/h264dec.c:1048
> > 
> > (gdb) p avctx
> > $1 = (AVCodecContext *) 0x6447b00
> > (gdb) p avctx->hw_frames_ctx
> > $2 = (AVBufferRef *) 0x0
> > 
> > v2: check for free_frame_priv (Hendrik)
> > v3: return EINVAL (Christoph Reiter)
> > 
> > See[1]: https://github.com/msys2/MINGW-packages/pull/19050
> > Fixes: be07145109 ("avcodec: add AVHWAccel.free_frame_priv
> > callback")
> > CC: Lynne <dev@lynne.ee>
> > CC: Christoph Reiter <reiter.christoph@gmail.com>
> > Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> > ---
> 
> The latest change itself looks fine to me, I would however prefer if
> the commit message would contain a bit more text and a bit less gdb
> dump.
> 
> Maybe something like this (quick suggestion, for title and body):
> 
> avcodec: validate hw_frames_ctx is available when
> AVHWAccel.free_frame_priv is used
> 
> Validate that a hw_frames_ctx is available before using it for the
> AVHWAccel.free_frame_priv callback, and don't require it to be
> present when
> the callback is not in use by the HWAccel.

I am fine with that. Thank you for the text. Added with minor change in
title to shorten it. See v4 patch.

> 
> ---
> 
> Feel free to augment and reword, but I don't think the gdb dump
> offers
> much info that couldn't be conveyed more clearly in a few words that
> mention the absence of hw_frame_ctx.
> 
> - Hendrik
> _______________________________________________
> 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".
_______________________________________________
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] 14+ messages in thread

* Re: [FFmpeg-devel] [PATCH v4] avcodec/decode: validate hw_frames_ctx when AVHWAccel.free_frame_priv is used
       [not found] ` <1700541472-12254-1-git-send-email-dmitry.v.rogozhkin@intel.com-NjkA62m----9>
@ 2023-11-21  5:17   ` Lynne
  2023-11-21  6:02     ` Rogozhkin, Dmitry V
  0 siblings, 1 reply; 14+ messages in thread
From: Lynne @ 2023-11-21  5:17 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Nov 21, 2023, 05:38 by dmitry.v.rogozhkin-at-intel.com@ffmpeg.org:

> Validate that a hw_frames_ctx is available before using it for
> the AVHWAccel.free_frame_priv callback, and don't require it to
> be present when the callback is not in use by the HWAccel.
>
> v2: check for free_frame_priv (Hendrik)
> v3: return EINVAL (Christoph Reiter)
> v4: better commit message (Hendrik)
>
> See[1]: https://github.com/msys2/MINGW-packages/pull/19050
> Fixes: be07145109 ("avcodec: add AVHWAccel.free_frame_priv callback")
> CC: Lynne <dev@lynne.ee>
> CC: Christoph Reiter <reiter.christoph@gmail.com>
> Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> ---
>  libavcodec/decode.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index ad39021..50c3995 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1838,17 +1838,25 @@ int ff_copy_palette(void *dst, const AVPacket *src, void *logctx)
>  int ff_hwaccel_frame_priv_alloc(AVCodecContext *avctx, void **hwaccel_picture_private)
>  {
>  const FFHWAccel *hwaccel = ffhwaccel(avctx->hwaccel);
> -    AVHWFramesContext *frames_ctx;
>  
>  if (!hwaccel || !hwaccel->frame_priv_data_size)
>  return 0;
>  
>  av_assert0(!*hwaccel_picture_private);
>  
> -    frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
> -    *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel->frame_priv_data_size, 0,
> -                                                      frames_ctx->device_ctx,
> -                                                      hwaccel->free_frame_priv);
> +    if (hwaccel->free_frame_priv) {
> +        AVHWFramesContext *frames_ctx;
> +
> +        if (!avctx->hw_frames_ctx)
> +            return AVERROR(EINVAL);
> +
> +        *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel->frame_priv_data_size, 0,
> +                                                          frames_ctx->device_ctx,
> +                                                          hwaccel->free_frame_priv);
> +    } else {
> +        *hwaccel_picture_private = ff_refstruct_allocz(hwaccel->frame_priv_data_size);
> +    }
> +
>  if (!*hwaccel_picture_private)
>  return AVERROR(ENOMEM);
>

You never set frames_ctx in this patch.
_______________________________________________
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] 14+ messages in thread

* [FFmpeg-devel] [PATCH v5] avcodec/decode: validate hw_frames_ctx when AVHWAccel.free_frame_priv is used
  2023-11-14 18:22 [FFmpeg-devel] [PATCH] avcodec/decode: guard against NULL hw_frames_ctx Dmitry Rogozhkin
                   ` (4 preceding siblings ...)
       [not found] ` <1700541472-12254-1-git-send-email-dmitry.v.rogozhkin@intel.com-NjkA62m----9>
@ 2023-11-21  5:57 ` Dmitry Rogozhkin
  5 siblings, 0 replies; 14+ messages in thread
From: Dmitry Rogozhkin @ 2023-11-21  5:57 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Christoph Reiter, Dmitry Rogozhkin, Lynne

Validate that a hw_frames_ctx is available before using it for
the AVHWAccel.free_frame_priv callback, and don't require it to
be present when the callback is not in use by the HWAccel.

v2: check for free_frame_priv (Hendrik)
v3: return EINVAL (Christoph Reiter)
v4: better commit message (Hendrik)
v5: fix typo with missed frames_ctx (Lynne)

See[1]: https://github.com/msys2/MINGW-packages/pull/19050
Fixes: be07145109 ("avcodec: add AVHWAccel.free_frame_priv callback")
CC: Lynne <dev@lynne.ee>
CC: Christoph Reiter <reiter.christoph@gmail.com>
Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
---
 libavcodec/decode.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index ad39021..2cfb3fc 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1838,17 +1838,26 @@ int ff_copy_palette(void *dst, const AVPacket *src, void *logctx)
 int ff_hwaccel_frame_priv_alloc(AVCodecContext *avctx, void **hwaccel_picture_private)
 {
     const FFHWAccel *hwaccel = ffhwaccel(avctx->hwaccel);
-    AVHWFramesContext *frames_ctx;
 
     if (!hwaccel || !hwaccel->frame_priv_data_size)
         return 0;
 
     av_assert0(!*hwaccel_picture_private);
 
-    frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
-    *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel->frame_priv_data_size, 0,
-                                                      frames_ctx->device_ctx,
-                                                      hwaccel->free_frame_priv);
+    if (hwaccel->free_frame_priv) {
+        AVHWFramesContext *frames_ctx;
+
+        if (!avctx->hw_frames_ctx)
+            return AVERROR(EINVAL);
+
+        frames_ctx = (AVHWFramesContext *) avctx->hw_frames_ctx->data;
+        *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel->frame_priv_data_size, 0,
+                                                          frames_ctx->device_ctx,
+                                                          hwaccel->free_frame_priv);
+    } else {
+        *hwaccel_picture_private = ff_refstruct_allocz(hwaccel->frame_priv_data_size);
+    }
+
     if (!*hwaccel_picture_private)
         return AVERROR(ENOMEM);
 
-- 
1.8.3.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".

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

* Re: [FFmpeg-devel] [PATCH v4] avcodec/decode: validate hw_frames_ctx when AVHWAccel.free_frame_priv is used
  2023-11-21  5:17   ` Lynne
@ 2023-11-21  6:02     ` Rogozhkin, Dmitry V
  2023-11-22  4:08       ` Lynne
  0 siblings, 1 reply; 14+ messages in thread
From: Rogozhkin, Dmitry V @ 2023-11-21  6:02 UTC (permalink / raw)
  To: ffmpeg-devel

On Tue, 2023-11-21 at 06:17 +0100, Lynne wrote:
> Nov 21, 2023, 05:38 by dmitry.v.rogozhkin-at-intel.com@ffmpeg.org:
> 
> > Validate that a hw_frames_ctx is available before using it for
> > the AVHWAccel.free_frame_priv callback, and don't require it to
> > be present when the callback is not in use by the HWAccel.
> > 
> > v2: check for free_frame_priv (Hendrik)
> > v3: return EINVAL (Christoph Reiter)
> > v4: better commit message (Hendrik)
> > 
> > See[1]: https://github.com/msys2/MINGW-packages/pull/19050
> > Fixes: be07145109 ("avcodec: add AVHWAccel.free_frame_priv
> > callback")
> > CC: Lynne <dev@lynne.ee>
> > CC: Christoph Reiter <reiter.christoph@gmail.com>
> > Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> > ---
> >  libavcodec/decode.c | 18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> > index ad39021..50c3995 100644
> > --- a/libavcodec/decode.c
> > +++ b/libavcodec/decode.c
> > @@ -1838,17 +1838,25 @@ int ff_copy_palette(void *dst, const
> > AVPacket *src, void *logctx)
> >  int ff_hwaccel_frame_priv_alloc(AVCodecContext *avctx, void
> > **hwaccel_picture_private)
> >  {
> >  const FFHWAccel *hwaccel = ffhwaccel(avctx->hwaccel);
> > -    AVHWFramesContext *frames_ctx;
> >  
> >  if (!hwaccel || !hwaccel->frame_priv_data_size)
> >  return 0;
> >  
> >  av_assert0(!*hwaccel_picture_private);
> >  
> > -    frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
> > -    *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel-
> > >frame_priv_data_size, 0,
> > -                                                      frames_ctx-
> > >device_ctx,
> > -                                                      hwaccel-
> > >free_frame_priv);
> > +    if (hwaccel->free_frame_priv) {
> > +        AVHWFramesContext *frames_ctx;
> > +
> > +        if (!avctx->hw_frames_ctx)
> > +            return AVERROR(EINVAL);
> > +
> > +        *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel-
> > >frame_priv_data_size, 0,
> > +                                                          frames_c
> > tx->device_ctx,
> > +                                                          hwaccel-
> > >free_frame_priv);
> > +    } else {
> > +        *hwaccel_picture_private = ff_refstruct_allocz(hwaccel-
> > >frame_priv_data_size);
> > +    }
> > +
> >  if (!*hwaccel_picture_private)
> >  return AVERROR(ENOMEM);
> > 
> 
> You never set frames_ctx in this patch.

Thank you for the catch. Yep, I did a type doing v3. Fixed now. See v5.

> _______________________________________________
> 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".
_______________________________________________
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] 14+ messages in thread

* Re: [FFmpeg-devel] [PATCH v4] avcodec/decode: validate hw_frames_ctx when AVHWAccel.free_frame_priv is used
  2023-11-21  6:02     ` Rogozhkin, Dmitry V
@ 2023-11-22  4:08       ` Lynne
  0 siblings, 0 replies; 14+ messages in thread
From: Lynne @ 2023-11-22  4:08 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Nov 21, 2023, 07:02 by dmitry.v.rogozhkin-at-intel.com@ffmpeg.org:

> On Tue, 2023-11-21 at 06:17 +0100, Lynne wrote:
>
>> Nov 21, 2023, 05:38 by dmitry.v.rogozhkin-at-intel.com@ffmpeg.org:
>>
>> > Validate that a hw_frames_ctx is available before using it for
>> > the AVHWAccel.free_frame_priv callback, and don't require it to
>> > be present when the callback is not in use by the HWAccel.
>> > 
>> > v2: check for free_frame_priv (Hendrik)
>> > v3: return EINVAL (Christoph Reiter)
>> > v4: better commit message (Hendrik)
>> > 
>> > See[1]: https://github.com/msys2/MINGW-packages/pull/19050
>> > Fixes: be07145109 ("avcodec: add AVHWAccel.free_frame_priv
>> > callback")
>> > CC: Lynne <dev@lynne.ee>
>> > CC: Christoph Reiter <reiter.christoph@gmail.com>
>> > Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
>> > ---
>> >  libavcodec/decode.c | 18 +++++++++++++-----
>> >  1 file changed, 13 insertions(+), 5 deletions(-)
>> > 
>> > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
>> > index ad39021..50c3995 100644
>> > --- a/libavcodec/decode.c
>> > +++ b/libavcodec/decode.c
>> > @@ -1838,17 +1838,25 @@ int ff_copy_palette(void *dst, const
>> > AVPacket *src, void *logctx)
>> >  int ff_hwaccel_frame_priv_alloc(AVCodecContext *avctx, void
>> > **hwaccel_picture_private)
>> >  {
>> >  const FFHWAccel *hwaccel = ffhwaccel(avctx->hwaccel);
>> > -    AVHWFramesContext *frames_ctx;
>> > 
>> >  if (!hwaccel || !hwaccel->frame_priv_data_size)
>> >  return 0;
>> > 
>> >  av_assert0(!*hwaccel_picture_private);
>> > 
>> > -    frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
>> > -    *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel-
>> > >frame_priv_data_size, 0,
>> > -                                                      frames_ctx-
>> > >device_ctx,
>> > -                                                      hwaccel-
>> > >free_frame_priv);
>> > +    if (hwaccel->free_frame_priv) {
>> > +        AVHWFramesContext *frames_ctx;
>> > +
>> > +        if (!avctx->hw_frames_ctx)
>> > +            return AVERROR(EINVAL);
>> > +
>> > +        *hwaccel_picture_private = ff_refstruct_alloc_ext(hwaccel-
>> > >frame_priv_data_size, 0,
>> > +                                                          frames_c
>> > tx->device_ctx,
>> > +                                                          hwaccel-
>> > >free_frame_priv);
>> > +    } else {
>> > +        *hwaccel_picture_private = ff_refstruct_allocz(hwaccel-
>> > >frame_priv_data_size);
>> > +    }
>> > +
>> >  if (!*hwaccel_picture_private)
>> >  return AVERROR(ENOMEM);
>> > 
>>
>> You never set frames_ctx in this patch.
>>
>
> Thank you for the catch. Yep, I did a type doing v3. Fixed now. See v5.
>

Pushed, thanks
_______________________________________________
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] 14+ messages in thread

end of thread, other threads:[~2023-11-22  4:08 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-14 18:22 [FFmpeg-devel] [PATCH] avcodec/decode: guard against NULL hw_frames_ctx Dmitry Rogozhkin
2023-11-14 18:32 ` Hendrik Leppkes
2023-11-15 15:28   ` Rogozhkin, Dmitry V
2023-11-14 19:14 ` [FFmpeg-devel] [PATCH v2] " Dmitry Rogozhkin
2023-11-17 15:41   ` Rogozhkin, Dmitry V
2023-11-17 17:03 ` [FFmpeg-devel] [PATCH v3] " Dmitry Rogozhkin
2023-11-19 10:10   ` Sebastian Ramacher
2023-11-19 15:29   ` Hendrik Leppkes
2023-11-21  4:45     ` Rogozhkin, Dmitry V
2023-11-21  4:37 ` [FFmpeg-devel] [PATCH v4] avcodec/decode: validate hw_frames_ctx when AVHWAccel.free_frame_priv is used Dmitry Rogozhkin
     [not found] ` <1700541472-12254-1-git-send-email-dmitry.v.rogozhkin@intel.com-NjkA62m----9>
2023-11-21  5:17   ` Lynne
2023-11-21  6:02     ` Rogozhkin, Dmitry V
2023-11-22  4:08       ` Lynne
2023-11-21  5:57 ` [FFmpeg-devel] [PATCH v5] " Dmitry Rogozhkin

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git