| Message ID | 20201011000918.1137879-1-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/vp9: Fix stack-buffer overflow with VP9 VDPAU available | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | warning | Make failed |
Andreas Rheinhardt: > ccca62ef991f0a47dfa30c3e822d91294b8afe4c added new VP9 VDPAU profiles > and as a consequence AV_PIX_FMT_VDPAU can now be twice in the list of > pixel formats used for format negotiation by ff_thread_get_format(); yet > there is only one entry in said list reserved for VDPAU, leading to a > stack-buffer overflow. This commit fixes this by making sure that > AV_PIX_FMT_VDPAU will not occur twice in said list. > > Fixes Coverity ticket 1468046. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > Other solutions would be > a) Counting CONFIG_VP9_VDPAU_HWACCEL twice for the size of the pix_fmts > array (as is already done for D3D11VA). But this would leave > AV_PIX_FMT_VDPAU twice in the list which seems wrong. > b) Adding a break inside the #if CONFIG_VP9_VDPAU_HWACCEL block for > YUV420P. But then the other hardware accelerations won't ever get a > chance. > c) Let AV_PIX_FMT_YUV420P directly fall through to AV_PIX_FMT_YUV420P10 > without adding a pixel format there at all; AV_PIX_FMT_VDPAU would then > be added at the end, changing the current priorities, unless it would > be moved to the beginning of the AV_PIX_FMT_YUV420P10 block. > Based upon review from Philip Langdale on IRC solution c) (without moving) has been implemented. > libavcodec/vp9.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c > index 8b89fd68e2..d04e9fc062 100644 > --- a/libavcodec/vp9.c > +++ b/libavcodec/vp9.c > @@ -225,7 +225,8 @@ static int update_size(AVCodecContext *avctx, int w, int h) > *fmtp++ = AV_PIX_FMT_VAAPI; > #endif > #if CONFIG_VP9_VDPAU_HWACCEL > - *fmtp++ = AV_PIX_FMT_VDPAU; > + if (s->pix_fmt != AV_PIX_FMT_YUV420P) > + *fmtp++ = AV_PIX_FMT_VDPAU; > #endif > break; > case AV_PIX_FMT_YUV420P12: >
diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c index 8b89fd68e2..d04e9fc062 100644 --- a/libavcodec/vp9.c +++ b/libavcodec/vp9.c @@ -225,7 +225,8 @@ static int update_size(AVCodecContext *avctx, int w, int h) *fmtp++ = AV_PIX_FMT_VAAPI; #endif #if CONFIG_VP9_VDPAU_HWACCEL - *fmtp++ = AV_PIX_FMT_VDPAU; + if (s->pix_fmt != AV_PIX_FMT_YUV420P) + *fmtp++ = AV_PIX_FMT_VDPAU; #endif break; case AV_PIX_FMT_YUV420P12:
ccca62ef991f0a47dfa30c3e822d91294b8afe4c added new VP9 VDPAU profiles and as a consequence AV_PIX_FMT_VDPAU can now be twice in the list of pixel formats used for format negotiation by ff_thread_get_format(); yet there is only one entry in said list reserved for VDPAU, leading to a stack-buffer overflow. This commit fixes this by making sure that AV_PIX_FMT_VDPAU will not occur twice in said list. Fixes Coverity ticket 1468046. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- Other solutions would be a) Counting CONFIG_VP9_VDPAU_HWACCEL twice for the size of the pix_fmts array (as is already done for D3D11VA). But this would leave AV_PIX_FMT_VDPAU twice in the list which seems wrong. b) Adding a break inside the #if CONFIG_VP9_VDPAU_HWACCEL block for YUV420P. But then the other hardware accelerations won't ever get a chance. c) Let AV_PIX_FMT_YUV420P directly fall through to AV_PIX_FMT_YUV420P10 without adding a pixel format there at all; AV_PIX_FMT_VDPAU would then be added at the end, changing the current priorities, unless it would be moved to the beginning of the AV_PIX_FMT_YUV420P10 block. libavcodec/vp9.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)