* [FFmpeg-devel] [PATCH] avcodec/dpx: fix check of minimal data size for unpadded content @ 2022-10-19 9:47 Jerome Martinez 2023-03-14 10:19 ` Jerome Martinez 0 siblings, 1 reply; 3+ messages in thread From: Jerome Martinez @ 2022-10-19 9:47 UTC (permalink / raw) To: FFmpeg development discussions and patches stride value is not relevant with unpadded content and the total count of pixels (width x height) must be used instead of the rounding based on width only then multiplied by height unpadded_10bit value computing is moved sooner in the code in order to be able to use it during computing of minimal content size Fix 'Overread buffer' error when the content is not lucky enough to have (enough) padding bytes at the end for not being rejected by the formula based on the stride value Signed-off-by: Jerome Martinez <jerome@mediaarea.net> --- libavcodec/dpx.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/libavcodec/dpx.c b/libavcodec/dpx.c index 4f50608..d4699f6 100644 --- a/libavcodec/dpx.c +++ b/libavcodec/dpx.c @@ -476,14 +476,30 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *p, avctx->colorspace = AVCOL_SPC_RGB; } + av_strlcpy(creator, avpkt->data + 160, 100); + creator[100] = '\0'; + av_dict_set(&p->metadata, "Creator", creator, 0); + + av_strlcpy(input_device, avpkt->data + 1556, 32); + input_device[32] = '\0'; + av_dict_set(&p->metadata, "Input Device", input_device, 0); + + // Some devices do not pad 10bit samples to whole 32bit words per row + if (!memcmp(input_device, "Scanity", 7) || + !memcmp(creator, "Lasergraphics Inc.", 18)) { + unpadded_10bit = 1; + } + // Table 3c: Runs will always break at scan line boundaries. Packing // will always break to the next 32-bit word at scan-line boundaries. // Unfortunately, the encoder produced invalid files, so attempt // to detect it + // Also handle special case with unpadded content need_align = FFALIGN(stride, 4); - if (need_align*avctx->height + (int64_t)offset > avpkt->size) { + if (need_align*avctx->height + (int64_t)offset > avpkt->size && + (!unpadded_10bit || (avctx->width * avctx->height * elements + 2) / 3 * 4 + (int64_t)offset > avpkt->size)) { // Alignment seems unappliable, try without - if (stride*avctx->height + (int64_t)offset > avpkt->size) { + if (stride*avctx->height + (int64_t)offset > avpkt->size || unpadded_10bit) { av_log(avctx, AV_LOG_ERROR, "Overread buffer. Invalid header?\n"); return AVERROR_INVALIDDATA; } else { @@ -609,20 +625,6 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *p, if ((ret = ff_get_buffer(avctx, p, 0)) < 0) return ret; - av_strlcpy(creator, avpkt->data + 160, 100); - creator[100] = '\0'; - av_dict_set(&p->metadata, "Creator", creator, 0); - - av_strlcpy(input_device, avpkt->data + 1556, 32); - input_device[32] = '\0'; - av_dict_set(&p->metadata, "Input Device", input_device, 0); - - // Some devices do not pad 10bit samples to whole 32bit words per row - if (!memcmp(input_device, "Scanity", 7) || - !memcmp(creator, "Lasergraphics Inc.", 18)) { - unpadded_10bit = 1; - } - // Move pointer to offset from start of file buf = avpkt->data + offset; -- 2.25.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". ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/dpx: fix check of minimal data size for unpadded content 2022-10-19 9:47 [FFmpeg-devel] [PATCH] avcodec/dpx: fix check of minimal data size for unpadded content Jerome Martinez @ 2023-03-14 10:19 ` Jerome Martinez 2023-04-08 18:04 ` Marton Balint 0 siblings, 1 reply; 3+ messages in thread From: Jerome Martinez @ 2023-03-14 10:19 UTC (permalink / raw) To: ffmpeg-devel [-- Attachment #1: Type: text/plain, Size: 3797 bytes --] Please consider the attached patch. Before: "Overread buffer. Invalid header?" despite that all bytes are there (the precheck is wrong, not the parsing after the precheck) After: transcoding is fine A (zeroed) sample file is available at https://trac.ffmpeg.org/ticket/10259 Jérôme On 19/10/2022 11:47, Jerome Martinez wrote: > stride value is not relevant with unpadded content and the total count > of pixels (width x height) must be used instead of the rounding based > on width only then multiplied by height > > unpadded_10bit value computing is moved sooner in the code in order to > be able to use it during computing of minimal content size > > Fix 'Overread buffer' error when the content is not lucky enough to > have (enough) padding bytes at the end for not being rejected by the > formula based on the stride value > > Signed-off-by: Jerome Martinez <jerome@mediaarea.net> > --- > libavcodec/dpx.c | 34 ++++++++++++++++++---------------- > 1 file changed, 18 insertions(+), 16 deletions(-) > > diff --git a/libavcodec/dpx.c b/libavcodec/dpx.c > index 4f50608..d4699f6 100644 > --- a/libavcodec/dpx.c > +++ b/libavcodec/dpx.c > @@ -476,14 +476,30 @@ static int decode_frame(AVCodecContext *avctx, > AVFrame *p, > avctx->colorspace = AVCOL_SPC_RGB; > } > > + av_strlcpy(creator, avpkt->data + 160, 100); > + creator[100] = '\0'; > + av_dict_set(&p->metadata, "Creator", creator, 0); > + > + av_strlcpy(input_device, avpkt->data + 1556, 32); > + input_device[32] = '\0'; > + av_dict_set(&p->metadata, "Input Device", input_device, 0); > + > + // Some devices do not pad 10bit samples to whole 32bit words per > row > + if (!memcmp(input_device, "Scanity", 7) || > + !memcmp(creator, "Lasergraphics Inc.", 18)) { > + unpadded_10bit = 1; > + } > + > // Table 3c: Runs will always break at scan line boundaries. Packing > // will always break to the next 32-bit word at scan-line > boundaries. > // Unfortunately, the encoder produced invalid files, so attempt > // to detect it > + // Also handle special case with unpadded content > need_align = FFALIGN(stride, 4); > - if (need_align*avctx->height + (int64_t)offset > avpkt->size) { > + if (need_align*avctx->height + (int64_t)offset > avpkt->size && > + (!unpadded_10bit || (avctx->width * avctx->height * elements > + 2) / 3 * 4 + (int64_t)offset > avpkt->size)) { > // Alignment seems unappliable, try without > - if (stride*avctx->height + (int64_t)offset > avpkt->size) { > + if (stride*avctx->height + (int64_t)offset > avpkt->size || > unpadded_10bit) { > av_log(avctx, AV_LOG_ERROR, "Overread buffer. Invalid > header?\n"); > return AVERROR_INVALIDDATA; > } else { > @@ -609,20 +625,6 @@ static int decode_frame(AVCodecContext *avctx, > AVFrame *p, > if ((ret = ff_get_buffer(avctx, p, 0)) < 0) > return ret; > > - av_strlcpy(creator, avpkt->data + 160, 100); > - creator[100] = '\0'; > - av_dict_set(&p->metadata, "Creator", creator, 0); > - > - av_strlcpy(input_device, avpkt->data + 1556, 32); > - input_device[32] = '\0'; > - av_dict_set(&p->metadata, "Input Device", input_device, 0); > - > - // Some devices do not pad 10bit samples to whole 32bit words per > row > - if (!memcmp(input_device, "Scanity", 7) || > - !memcmp(creator, "Lasergraphics Inc.", 18)) { > - unpadded_10bit = 1; > - } > - > // Move pointer to offset from start of file > buf = avpkt->data + offset; > [-- Attachment #2: 0001-avcodec-dpx-fix-check-of-minimal-data-size-for-unpad.patch --] [-- Type: text/plain, Size: 3301 bytes --] From 21c21373ca576f1dff05f952c17275957b9388bd Mon Sep 17 00:00:00 2001 From: Jerome Martinez <jerome@mediaarea.net> Date: Wed, 19 Oct 2022 11:37:34 +0200 Subject: [PATCH] avcodec/dpx: fix check of minimal data size for unpadded content stride value is not relevant with unpadded content and the total count of pixels (width x height) must be used instead of the rounding based on width only then multiplied by height unpadded_10bit value computing is moved sooner in the code in order to be able to use it during computing of minimal content size Fix 'Overread buffer' error when the content is not lucky enough to have (enough) padding bytes at the end for not being rejected by the formula based on the stride value --- libavcodec/dpx.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/libavcodec/dpx.c b/libavcodec/dpx.c index 4f50608461..d4699f65fc 100644 --- a/libavcodec/dpx.c +++ b/libavcodec/dpx.c @@ -476,14 +476,30 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *p, avctx->colorspace = AVCOL_SPC_RGB; } + av_strlcpy(creator, avpkt->data + 160, 100); + creator[100] = '\0'; + av_dict_set(&p->metadata, "Creator", creator, 0); + + av_strlcpy(input_device, avpkt->data + 1556, 32); + input_device[32] = '\0'; + av_dict_set(&p->metadata, "Input Device", input_device, 0); + + // Some devices do not pad 10bit samples to whole 32bit words per row + if (!memcmp(input_device, "Scanity", 7) || + !memcmp(creator, "Lasergraphics Inc.", 18)) { + unpadded_10bit = 1; + } + // Table 3c: Runs will always break at scan line boundaries. Packing // will always break to the next 32-bit word at scan-line boundaries. // Unfortunately, the encoder produced invalid files, so attempt // to detect it + // Also handle special case with unpadded content need_align = FFALIGN(stride, 4); - if (need_align*avctx->height + (int64_t)offset > avpkt->size) { + if (need_align*avctx->height + (int64_t)offset > avpkt->size && + (!unpadded_10bit || (avctx->width * avctx->height * elements + 2) / 3 * 4 + (int64_t)offset > avpkt->size)) { // Alignment seems unappliable, try without - if (stride*avctx->height + (int64_t)offset > avpkt->size) { + if (stride*avctx->height + (int64_t)offset > avpkt->size || unpadded_10bit) { av_log(avctx, AV_LOG_ERROR, "Overread buffer. Invalid header?\n"); return AVERROR_INVALIDDATA; } else { @@ -609,20 +625,6 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *p, if ((ret = ff_get_buffer(avctx, p, 0)) < 0) return ret; - av_strlcpy(creator, avpkt->data + 160, 100); - creator[100] = '\0'; - av_dict_set(&p->metadata, "Creator", creator, 0); - - av_strlcpy(input_device, avpkt->data + 1556, 32); - input_device[32] = '\0'; - av_dict_set(&p->metadata, "Input Device", input_device, 0); - - // Some devices do not pad 10bit samples to whole 32bit words per row - if (!memcmp(input_device, "Scanity", 7) || - !memcmp(creator, "Lasergraphics Inc.", 18)) { - unpadded_10bit = 1; - } - // Move pointer to offset from start of file buf = avpkt->data + offset; -- 2.13.3.windows.1 [-- Attachment #3: 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". ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/dpx: fix check of minimal data size for unpadded content 2023-03-14 10:19 ` Jerome Martinez @ 2023-04-08 18:04 ` Marton Balint 0 siblings, 0 replies; 3+ messages in thread From: Marton Balint @ 2023-04-08 18:04 UTC (permalink / raw) To: FFmpeg development discussions and patches On Tue, 14 Mar 2023, Jerome Martinez wrote: > Please consider the attached patch. > > Before: "Overread buffer. Invalid header?" despite that all bytes are there > (the precheck is wrong, not the parsing after the precheck) > After: transcoding is fine > > A (zeroed) sample file is available at https://trac.ffmpeg.org/ticket/10259 Will apply with a minor fix. Regards, Marton > > Jérôme > > On 19/10/2022 11:47, Jerome Martinez wrote: >> stride value is not relevant with unpadded content and the total count of >> pixels (width x height) must be used instead of the rounding based on >> width only then multiplied by height >> >> unpadded_10bit value computing is moved sooner in the code in order to be >> able to use it during computing of minimal content size >> >> Fix 'Overread buffer' error when the content is not lucky enough to have >> (enough) padding bytes at the end for not being rejected by the formula >> based on the stride value >> >> Signed-off-by: Jerome Martinez <jerome@mediaarea.net> >> --- >> libavcodec/dpx.c | 34 ++++++++++++++++++---------------- >> 1 file changed, 18 insertions(+), 16 deletions(-) >> >> diff --git a/libavcodec/dpx.c b/libavcodec/dpx.c >> index 4f50608..d4699f6 100644 >> --- a/libavcodec/dpx.c >> +++ b/libavcodec/dpx.c >> @@ -476,14 +476,30 @@ static int decode_frame(AVCodecContext *avctx, >> AVFrame *p, >> avctx->colorspace = AVCOL_SPC_RGB; >> } >> >> + av_strlcpy(creator, avpkt->data + 160, 100); >> + creator[100] = '\0'; >> + av_dict_set(&p->metadata, "Creator", creator, 0); >> + >> + av_strlcpy(input_device, avpkt->data + 1556, 32); >> + input_device[32] = '\0'; >> + av_dict_set(&p->metadata, "Input Device", input_device, 0); >> + >> + // Some devices do not pad 10bit samples to whole 32bit words per row >> + if (!memcmp(input_device, "Scanity", 7) || >> + !memcmp(creator, "Lasergraphics Inc.", 18)) { >> + unpadded_10bit = 1; >> + } >> + >> // Table 3c: Runs will always break at scan line boundaries. Packing >> // will always break to the next 32-bit word at scan-line boundaries. >> // Unfortunately, the encoder produced invalid files, so attempt >> // to detect it >> + // Also handle special case with unpadded content >> need_align = FFALIGN(stride, 4); >> - if (need_align*avctx->height + (int64_t)offset > avpkt->size) { >> + if (need_align*avctx->height + (int64_t)offset > avpkt->size && >> + (!unpadded_10bit || (avctx->width * avctx->height * elements + 2) >> / 3 * 4 + (int64_t)offset > avpkt->size)) { >> // Alignment seems unappliable, try without >> - if (stride*avctx->height + (int64_t)offset > avpkt->size) { >> + if (stride*avctx->height + (int64_t)offset > avpkt->size || >> unpadded_10bit) { >> av_log(avctx, AV_LOG_ERROR, "Overread buffer. Invalid >> header?\n"); >> return AVERROR_INVALIDDATA; >> } else { >> @@ -609,20 +625,6 @@ static int decode_frame(AVCodecContext *avctx, >> AVFrame *p, >> if ((ret = ff_get_buffer(avctx, p, 0)) < 0) >> return ret; >> >> - av_strlcpy(creator, avpkt->data + 160, 100); >> - creator[100] = '\0'; >> - av_dict_set(&p->metadata, "Creator", creator, 0); >> - >> - av_strlcpy(input_device, avpkt->data + 1556, 32); >> - input_device[32] = '\0'; >> - av_dict_set(&p->metadata, "Input Device", input_device, 0); >> - >> - // Some devices do not pad 10bit samples to whole 32bit words per row >> - if (!memcmp(input_device, "Scanity", 7) || >> - !memcmp(creator, "Lasergraphics Inc.", 18)) { >> - unpadded_10bit = 1; >> - } >> - >> // Move pointer to offset from start of file >> buf = avpkt->data + offset; >> > _______________________________________________ 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". ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-04-08 18:05 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-19 9:47 [FFmpeg-devel] [PATCH] avcodec/dpx: fix check of minimal data size for unpadded content Jerome Martinez 2023-03-14 10:19 ` Jerome Martinez 2023-04-08 18:04 ` Marton Balint
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git