diff mbox series

[FFmpeg-devel] avutil/hwcontext_drm: Check ioctl in drm_map_frame() for failure

Message ID 20240520021944.361686-1-michael@niedermayer.cc
State New
Headers show
Series [FFmpeg-devel] avutil/hwcontext_drm: Check ioctl in drm_map_frame() for failure | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished

Commit Message

Michael Niedermayer May 20, 2024, 2:19 a.m. UTC
Fixes: CID1583742 Unchecked return value

Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavutil/hwcontext_drm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Andreas Rheinhardt May 20, 2024, 9:33 a.m. UTC | #1
Michael Niedermayer:
> Fixes: CID1583742 Unchecked return value
> 
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavutil/hwcontext_drm.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libavutil/hwcontext_drm.c b/libavutil/hwcontext_drm.c
> index 0847db09a08..e080c0597b8 100644
> --- a/libavutil/hwcontext_drm.c
> +++ b/libavutil/hwcontext_drm.c
> @@ -166,7 +166,10 @@ static int drm_map_frame(AVHWFramesContext *hwfc,
>  #if HAVE_LINUX_DMA_BUF_H
>          /* We're not checking for errors here because the kernel may not
>           * support the ioctl, in which case its okay to carry on */
> -        ioctl(desc->objects[i].fd, DMA_BUF_IOCTL_SYNC, &sync_start);
> +        if (ioctl(desc->objects[i].fd, DMA_BUF_IOCTL_SYNC, &sync_start) == -1) {
> +            err = AVERROR(errno);
> +            goto fail;
> +        }
>  #endif
>      }
>      map->nb_regions = i;

Did you read the comment above the code?

- Andreas
Michael Niedermayer May 20, 2024, 1:36 p.m. UTC | #2
On Mon, May 20, 2024 at 11:33:41AM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: CID1583742 Unchecked return value
> > 
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavutil/hwcontext_drm.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavutil/hwcontext_drm.c b/libavutil/hwcontext_drm.c
> > index 0847db09a08..e080c0597b8 100644
> > --- a/libavutil/hwcontext_drm.c
> > +++ b/libavutil/hwcontext_drm.c
> > @@ -166,7 +166,10 @@ static int drm_map_frame(AVHWFramesContext *hwfc,
> >  #if HAVE_LINUX_DMA_BUF_H
> >          /* We're not checking for errors here because the kernel may not
> >           * support the ioctl, in which case its okay to carry on */
> > -        ioctl(desc->objects[i].fd, DMA_BUF_IOCTL_SYNC, &sync_start);
> > +        if (ioctl(desc->objects[i].fd, DMA_BUF_IOCTL_SYNC, &sync_start) == -1) {
> > +            err = AVERROR(errno);
> > +            goto fail;
> > +        }
> >  #endif
> >      }
> >      map->nb_regions = i;
> 
> Did you read the comment above the code?

Apparently not

patch droped, will mark this as intentional

thx

[...]
Rémi Denis-Courmont May 20, 2024, 2:30 p.m. UTC | #3
Le 20 mai 2024 12:33:41 GMT+03:00, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> a écrit :
>Michael Niedermayer:
>> Fixes: CID1583742 Unchecked return value
>> 
>> Sponsored-by: Sovereign Tech Fund
>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>> ---
>>  libavutil/hwcontext_drm.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/libavutil/hwcontext_drm.c b/libavutil/hwcontext_drm.c
>> index 0847db09a08..e080c0597b8 100644
>> --- a/libavutil/hwcontext_drm.c
>> +++ b/libavutil/hwcontext_drm.c
>> @@ -166,7 +166,10 @@ static int drm_map_frame(AVHWFramesContext *hwfc,
>>  #if HAVE_LINUX_DMA_BUF_H
>>          /* We're not checking for errors here because the kernel may not
>>           * support the ioctl, in which case its okay to carry on */
>> -        ioctl(desc->objects[i].fd, DMA_BUF_IOCTL_SYNC, &sync_start);
>> +        if (ioctl(desc->objects[i].fd, DMA_BUF_IOCTL_SYNC, &sync_start) == -1) {
>> +            err = AVERROR(errno);
>> +            goto fail;
>> +        }
>>  #endif
>>      }
>>      map->nb_regions = i;
>
>Did you read the comment above the code?

It looks like this should check the errno value rather than flatly ignoring or not ignoring the error, not that I'd know.
diff mbox series

Patch

diff --git a/libavutil/hwcontext_drm.c b/libavutil/hwcontext_drm.c
index 0847db09a08..e080c0597b8 100644
--- a/libavutil/hwcontext_drm.c
+++ b/libavutil/hwcontext_drm.c
@@ -166,7 +166,10 @@  static int drm_map_frame(AVHWFramesContext *hwfc,
 #if HAVE_LINUX_DMA_BUF_H
         /* We're not checking for errors here because the kernel may not
          * support the ioctl, in which case its okay to carry on */
-        ioctl(desc->objects[i].fd, DMA_BUF_IOCTL_SYNC, &sync_start);
+        if (ioctl(desc->objects[i].fd, DMA_BUF_IOCTL_SYNC, &sync_start) == -1) {
+            err = AVERROR(errno);
+            goto fail;
+        }
 #endif
     }
     map->nb_regions = i;