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 |
Context | Check | Description |
---|---|---|
yinshiyou/make_loongarch64 | success | Make finished |
yinshiyou/make_fate_loongarch64 | success | Make fate finished |
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
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 [...]
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.
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 [...]
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;
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(-)