On 05.12.2015 04:02, Michael Niedermayer wrote:
> On Fri, Dec 04, 2015 at 03:14:21PM +0100, Andreas Cadhalpun wrote:
>> On 03.12.2015 15:48, Michael Niedermayer wrote:
>>> On Wed, Dec 02, 2015 at 10:00:13PM +0100, Andreas Cadhalpun wrote:
>>>> @@ -1293,14 +1296,16 @@ static int mjpeg_decode_scan(MJpegDecodeContext
>>>> *s, int nb_components, int Ah,
>>>> v = s->v_scount[i];
>>>> x = 0;
>>>> y = 0;
>>>> + h_shift = c ? chroma_h_shift: 0;
>>>> + v_shift = c ? chroma_v_shift: 0;
>>>> for (j = 0; j < n; j++) {
>>>> block_offset = (((linesize[c] * (v * mb_y + y) * 8) +
>>>> (h * mb_x + x) * 8 *
>>>> bytes_per_pixel) >> s->avctx->lowres);
>>>>
>>>> if (s->interlaced && s->bottom_field)
>>>> block_offset += linesize[c] >> 1;
>>>> - if ( 8*(h * mb_x + x) < s->width
>>>> - && 8*(v * mb_y + y) < s->height) {
>>>> + if ( 8*(h * mb_x + x) < (s->width + (1 << h_shift)
>>>> - 1) >> h_shift
>>>> + && 8*(v * mb_y + y) < (s->height + (1 << v_shift)
>>>> - 1) >> v_shift) {
>>>
>>> please move the w/h computation out of the block loop
>>> it stays the same for a component and does not need to be
>>> recalculated
>>> theres a loop above that fills data[] that can probably be used to
>>> fill w/h arrays
>>
>> OK, but since there are only two possible values, I don't think filling
>> arrays is necessary. Attached is an updated patch.
>
> couldnt there be a alpha plane too ?
Yes, fixed patch attached.
I also tested filling w/h arrays, but that turned out to be slightly slower.
Best regards,
Andreas
>From 7788195340e1d0e1206660f12f003f952da750a6 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
Date: Wed, 2 Dec 2015 21:52:23 +0100
Subject: [PATCH] mjpegdec: consider chroma subsampling in size check
If the chroma components are subsampled, smaller buffers are allocated
for them. In that case the maximal block_offset for the chroma
components is not as large as for the luma component.
This fixes out of bounds writes causing segmentation faults or memory
corruption.
Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
---
libavcodec/mjpegdec.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 4c9c82d..c812b86 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -1246,7 +1246,7 @@ static int mjpeg_decode_scan(MJpegDecodeContext *s, int nb_components, int Ah,
int mb_bitmask_size,
const AVFrame *reference)
{
- int i, mb_x, mb_y;
+ int i, mb_x, mb_y, chroma_h_shift, chroma_v_shift, chroma_width, chroma_height;
uint8_t *data[MAX_COMPONENTS];
const uint8_t *reference_data[MAX_COMPONENTS];
int linesize[MAX_COMPONENTS];
@@ -1263,6 +1263,11 @@ static int mjpeg_decode_scan(MJpegDecodeContext *s, int nb_components, int Ah,
s->restart_count = 0;
+ av_pix_fmt_get_chroma_sub_sample(s->avctx->pix_fmt, &chroma_h_shift,
+ &chroma_v_shift);
+ chroma_width = FF_CEIL_RSHIFT(s->width, chroma_h_shift);
+ chroma_height = FF_CEIL_RSHIFT(s->height, chroma_v_shift);
+
for (i = 0; i < nb_components; i++) {
int c = s->comp_index[i];
data[c] = s->picture_ptr->data[c];
@@ -1299,8 +1304,8 @@ static int mjpeg_decode_scan(MJpegDecodeContext *s, int nb_components, int Ah,
if (s->interlaced && s->bottom_field)
block_offset += linesize[c] >> 1;
- if ( 8*(h * mb_x + x) < s->width
- && 8*(v * mb_y + y) < s->height) {
+ if ( 8*(h * mb_x + x) < ((c == 1) || (c == 2) ? chroma_width : s->width)
+ && 8*(v * mb_y + y) < ((c == 1) || (c == 2) ? chroma_height : s->height)) {
ptr = data[c] + block_offset;
} else
ptr = NULL;
--
2.6.2
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
