diff mbox series

[FFmpeg-devel,04/17] avcodec/dxva2: initialize hr in ff_dxva2_common_end_frame()

Message ID 20240526235230.2876318-4-michael@niedermayer.cc
State New
Headers show
Series [FFmpeg-devel,01/17] avcodec/dxva2: Initialize dxva_size and check it | 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

Michael Niedermayer May 26, 2024, 11:52 p.m. UTC
Fixes: CID1591924 Uninitialized scalar variable
Fixes: CID1591938 Uninitialized scalar variable

Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/dxva2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andreas Rheinhardt June 2, 2024, 7:10 p.m. UTC | #1
Michael Niedermayer:
> Fixes: CID1591924 Uninitialized scalar variable
> Fixes: CID1591938 Uninitialized scalar variable
> 
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/dxva2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/dxva2.c b/libavcodec/dxva2.c
> index 1a33c8bbac7..22ecd5acafe 100644
> --- a/libavcodec/dxva2.c
> +++ b/libavcodec/dxva2.c
> @@ -906,7 +906,7 @@ int ff_dxva2_common_end_frame(AVCodecContext *avctx, AVFrame *frame,
>  #endif
>      DECODER_BUFFER_DESC             *buffer = NULL, *buffer_slice = NULL;
>      int result, runs = 0;
> -    HRESULT hr;
> +    HRESULT hr = -1;
>      unsigned type;
>      FFDXVASharedContext *sctx = DXVA_SHARED_CONTEXT(avctx);
>  

It seems to me that this (and other patches in this set) is not a real
fix of a bug. These functions are called either with a D3D11 pix fmt or
with AV_PIX_FMT_DXVA2_VLD, so these variables are initialized before
their use.

- Andreas
Michael Niedermayer June 2, 2024, 8:27 p.m. UTC | #2
On Sun, Jun 02, 2024 at 09:10:33PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: CID1591924 Uninitialized scalar variable
> > Fixes: CID1591938 Uninitialized scalar variable
> > 
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/dxva2.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/dxva2.c b/libavcodec/dxva2.c
> > index 1a33c8bbac7..22ecd5acafe 100644
> > --- a/libavcodec/dxva2.c
> > +++ b/libavcodec/dxva2.c
> > @@ -906,7 +906,7 @@ int ff_dxva2_common_end_frame(AVCodecContext *avctx, AVFrame *frame,
> >  #endif
> >      DECODER_BUFFER_DESC             *buffer = NULL, *buffer_slice = NULL;
> >      int result, runs = 0;
> > -    HRESULT hr;
> > +    HRESULT hr = -1;
> >      unsigned type;
> >      FFDXVASharedContext *sctx = DXVA_SHARED_CONTEXT(avctx);
> >  
> 
> It seems to me that this (and other patches in this set) is not a real
> fix of a bug. These functions are called either with a D3D11 pix fmt or
> with AV_PIX_FMT_DXVA2_VLD, so these variables are initialized before
> their use.

If they are called with another pixel format then its exploitable
maybe that cannot happen currently

But if or if not. Initializing these variables makes the code simply
more robust and also if it happens a NULL is easier to debug than
uninitialized variables. The assumtions that need to be true for
them to be initialized are not entirely trivial.

ill drop the patches from what i wanted to apply currently but i still
think they should be applied.
I can change the commit message if you can suggest something else ?

thx

[...]
Michael Niedermayer July 7, 2024, 7:48 p.m. UTC | #3
On Sun, Jun 02, 2024 at 10:27:55PM +0200, Michael Niedermayer wrote:
> On Sun, Jun 02, 2024 at 09:10:33PM +0200, Andreas Rheinhardt wrote:
> > Michael Niedermayer:
> > > Fixes: CID1591924 Uninitialized scalar variable
> > > Fixes: CID1591938 Uninitialized scalar variable
> > > 
> > > Sponsored-by: Sovereign Tech Fund
> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > ---
> > >  libavcodec/dxva2.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/libavcodec/dxva2.c b/libavcodec/dxva2.c
> > > index 1a33c8bbac7..22ecd5acafe 100644
> > > --- a/libavcodec/dxva2.c
> > > +++ b/libavcodec/dxva2.c
> > > @@ -906,7 +906,7 @@ int ff_dxva2_common_end_frame(AVCodecContext *avctx, AVFrame *frame,
> > >  #endif
> > >      DECODER_BUFFER_DESC             *buffer = NULL, *buffer_slice = NULL;
> > >      int result, runs = 0;
> > > -    HRESULT hr;
> > > +    HRESULT hr = -1;
> > >      unsigned type;
> > >      FFDXVASharedContext *sctx = DXVA_SHARED_CONTEXT(avctx);
> > >  
> > 
> > It seems to me that this (and other patches in this set) is not a real
> > fix of a bug. These functions are called either with a D3D11 pix fmt or
> > with AV_PIX_FMT_DXVA2_VLD, so these variables are initialized before
> > their use.
> 
> If they are called with another pixel format then its exploitable
> maybe that cannot happen currently
> 
> But if or if not. Initializing these variables makes the code simply
> more robust and also if it happens a NULL is easier to debug than
> uninitialized variables. The assumtions that need to be true for
> them to be initialized are not entirely trivial.
> 
> ill drop the patches from what i wanted to apply currently but i still
> think they should be applied.

> I can change the commit message if you can suggest something else ?

ping
does anyone object to the patches ?
does someone want a different commit message ? if so which ?

I will mark these issues as false positives, as they technically are

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/dxva2.c b/libavcodec/dxva2.c
index 1a33c8bbac7..22ecd5acafe 100644
--- a/libavcodec/dxva2.c
+++ b/libavcodec/dxva2.c
@@ -906,7 +906,7 @@  int ff_dxva2_common_end_frame(AVCodecContext *avctx, AVFrame *frame,
 #endif
     DECODER_BUFFER_DESC             *buffer = NULL, *buffer_slice = NULL;
     int result, runs = 0;
-    HRESULT hr;
+    HRESULT hr = -1;
     unsigned type;
     FFDXVASharedContext *sctx = DXVA_SHARED_CONTEXT(avctx);