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

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

[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;
-- 
2.45.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".

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

[Andreas Rheinhardt]

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

_______________________________________________
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".

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

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1460 bytes --]

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

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

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

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

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1737 bytes --]

On Mon, May 20, 2024 at 03:36:55PM +0200, Michael Niedermayer wrote:
> 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

theres a 2nd ioctl() call in the file (drm_unmap_frame()), that has no comment
and no check
Is that as intended or does it need some change ?
(its CID1583743 Unchecked return value)

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

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

[Rémi Denis-Courmont]

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.
_______________________________________________
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".