Skip to content

Commit

Permalink
Fix jpeg_skip_scanlines() segfault w/merged upsamp
Browse files Browse the repository at this point in the history
The additional segfault mentioned in #244 was due to the fact that
the merged upsamplers use a different private structure than the
non-merged upsamplers.  jpeg_skip_scanlines() was assuming the latter, so
when merged upsampling was enabled, jpeg_skip_scanlines() clobbered one
of the IDCT method pointers in the merged upsampler's private structure.

For reasons unknown, the test image in #441 did not encounter this
segfault (too small?), but it encountered an issue similar to the one
fixed in 5bc43c7, whereby it was
necessary to set up a dummy postprocessing function in
read_and_discard_scanlines() when merged upsampling was enabled.
Failing to do so caused either a segfault in merged_2v_upsample() (due
to a NULL pointer being passed to jcopy_sample_rows()) or an error
("Corrupt JPEG data: premature end of data segment"), depending on the
number of scanlines skipped and whether the first scanline skipped was
an odd- or even-numbered row.

Fixes #441
Fixes #244 (for real this time)
  • Loading branch information
dcommander committed Jul 24, 2020
1 parent c965dc7 commit 9120a24
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 53 deletions.
6 changes: 6 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@
platforms when using any of the YUV encoding/compression/decompression/decoding
methods in the TurboJPEG Java API.

2. Fixed segfaults or "Corrupt JPEG data: premature end of data segment" errors
in `jpeg_skip_scanlines()` that occurred when decompressing 4:2:2 or 4:2:0 JPEG
images using the merged (non-fancy) upsampling algorithms (that is, when
setting `cinfo.do_fancy_upsampling` to `FALSE`.) 2.0.0[6] was a similar fix,
but it did not cover all cases.


2.0.5
=====
Expand Down
72 changes: 64 additions & 8 deletions jdapistd.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This file was part of the Independent JPEG Group's software:
* Copyright (C) 1994-1996, Thomas G. Lane.
* libjpeg-turbo Modifications:
* Copyright (C) 2010, 2015-2018, D. R. Commander.
* Copyright (C) 2010, 2015-2018, 2020, D. R. Commander.
* Copyright (C) 2015, Google, Inc.
* For conditions of distribution and use, see the accompanying README.ijg
* file.
Expand All @@ -21,6 +21,8 @@
#include "jinclude.h"
#include "jdmainct.h"
#include "jdcoefct.h"
#include "jdmaster.h"
#include "jdmerge.h"
#include "jdsample.h"
#include "jmemsys.h"

Expand Down Expand Up @@ -304,6 +306,16 @@ noop_quantize(j_decompress_ptr cinfo, JSAMPARRAY input_buf,
}


/* Dummy postprocessing function used by jpeg_skip_scanlines() */
LOCAL(void)
noop_post_process (j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
JDIMENSION *in_row_group_ctr,
JDIMENSION in_row_groups_avail, JSAMPARRAY output_buf,
JDIMENSION *out_row_ctr, JDIMENSION out_rows_avail)
{
}


/*
* In some cases, it is best to call jpeg_read_scanlines() and discard the
* output, rather than skipping the scanlines, because this allows us to
Expand All @@ -316,11 +328,17 @@ LOCAL(void)
read_and_discard_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines)
{
JDIMENSION n;
my_master_ptr master = (my_master_ptr)cinfo->master;
void (*color_convert) (j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
JDIMENSION input_row, JSAMPARRAY output_buf,
int num_rows) = NULL;
void (*color_quantize) (j_decompress_ptr cinfo, JSAMPARRAY input_buf,
JSAMPARRAY output_buf, int num_rows) = NULL;
void (*post_process_data) (j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
JDIMENSION *in_row_group_ctr,
JDIMENSION in_row_groups_avail,
JSAMPARRAY output_buf, JDIMENSION *out_row_ctr,
JDIMENSION out_rows_avail) = NULL;

if (cinfo->cconvert && cinfo->cconvert->color_convert) {
color_convert = cinfo->cconvert->color_convert;
Expand All @@ -332,6 +350,12 @@ read_and_discard_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines)
cinfo->cquantize->color_quantize = noop_quantize;
}

if (master->using_merged_upsample && cinfo->post &&
cinfo->post->post_process_data) {
post_process_data = cinfo->post->post_process_data;
cinfo->post->post_process_data = noop_post_process;
}

for (n = 0; n < num_lines; n++)
jpeg_read_scanlines(cinfo, NULL, 1);

Expand All @@ -340,6 +364,9 @@ read_and_discard_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines)

if (color_quantize)
cinfo->cquantize->color_quantize = color_quantize;

if (post_process_data)
cinfo->post->post_process_data = post_process_data;
}


Expand Down Expand Up @@ -382,7 +409,7 @@ jpeg_skip_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines)
{
my_main_ptr main_ptr = (my_main_ptr)cinfo->main;
my_coef_ptr coef = (my_coef_ptr)cinfo->coef;
my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
my_master_ptr master = (my_master_ptr)cinfo->master;
JDIMENSION i, x;
int y;
JDIMENSION lines_per_iMCU_row, lines_left_in_iMCU_row, lines_after_iMCU_row;
Expand Down Expand Up @@ -445,8 +472,16 @@ jpeg_skip_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines)
main_ptr->buffer_full = FALSE;
main_ptr->rowgroup_ctr = 0;
main_ptr->context_state = CTX_PREPARE_FOR_IMCU;
upsample->next_row_out = cinfo->max_v_samp_factor;
upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
if (master->using_merged_upsample) {
my_merged_upsample_ptr upsample =
(my_merged_upsample_ptr)cinfo->upsample;
upsample->spare_full = FALSE;
upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
} else {
my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
upsample->next_row_out = cinfo->max_v_samp_factor;
upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
}
}

/* Skipping is much simpler when context rows are not required. */
Expand All @@ -458,8 +493,16 @@ jpeg_skip_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines)
cinfo->output_scanline += lines_left_in_iMCU_row;
main_ptr->buffer_full = FALSE;
main_ptr->rowgroup_ctr = 0;
upsample->next_row_out = cinfo->max_v_samp_factor;
upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
if (master->using_merged_upsample) {
my_merged_upsample_ptr upsample =
(my_merged_upsample_ptr)cinfo->upsample;
upsample->spare_full = FALSE;
upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
} else {
my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
upsample->next_row_out = cinfo->max_v_samp_factor;
upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
}
}
}

Expand Down Expand Up @@ -494,7 +537,14 @@ jpeg_skip_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines)
cinfo->output_iMCU_row += lines_to_skip / lines_per_iMCU_row;
increment_simple_rowgroup_ctr(cinfo, lines_to_read);
}
upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
if (master->using_merged_upsample) {
my_merged_upsample_ptr upsample =
(my_merged_upsample_ptr)cinfo->upsample;
upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
} else {
my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
}
return num_lines;
}

Expand Down Expand Up @@ -535,7 +585,13 @@ jpeg_skip_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines)
* bit odd, since "rows_to_go" seems to be redundantly keeping track of
* output_scanline.
*/
upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
if (master->using_merged_upsample) {
my_merged_upsample_ptr upsample = (my_merged_upsample_ptr)cinfo->upsample;
upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
} else {
my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
}

/* Always skip the requested number of lines. */
return num_lines;
Expand Down
46 changes: 9 additions & 37 deletions jdmerge.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Copyright (C) 1994-1996, Thomas G. Lane.
* libjpeg-turbo Modifications:
* Copyright 2009 Pierre Ossman <ossman@cendio.se> for Cendio AB
* Copyright (C) 2009, 2011, 2014-2015, D. R. Commander.
* Copyright (C) 2009, 2011, 2014-2015, 2020, D. R. Commander.
* Copyright (C) 2013, Linaro Limited.
* For conditions of distribution and use, see the accompanying README.ijg
* file.
Expand Down Expand Up @@ -40,41 +40,13 @@
#define JPEG_INTERNALS
#include "jinclude.h"
#include "jpeglib.h"
#include "jdmerge.h"
#include "jsimd.h"
#include "jconfigint.h"

#ifdef UPSAMPLE_MERGING_SUPPORTED


/* Private subobject */

typedef struct {
struct jpeg_upsampler pub; /* public fields */

/* Pointer to routine to do actual upsampling/conversion of one row group */
void (*upmethod) (j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
JDIMENSION in_row_group_ctr, JSAMPARRAY output_buf);

/* Private state for YCC->RGB conversion */
int *Cr_r_tab; /* => table for Cr to R conversion */
int *Cb_b_tab; /* => table for Cb to B conversion */
JLONG *Cr_g_tab; /* => table for Cr to G conversion */
JLONG *Cb_g_tab; /* => table for Cb to G conversion */

/* For 2:1 vertical sampling, we produce two output rows at a time.
* We need a "spare" row buffer to hold the second output row if the
* application provides just a one-row buffer; we also use the spare
* to discard the dummy last row if the image height is odd.
*/
JSAMPROW spare_row;
boolean spare_full; /* T if spare buffer is occupied */

JDIMENSION out_row_width; /* samples per output row */
JDIMENSION rows_to_go; /* counts rows remaining in image */
} my_upsampler;

typedef my_upsampler *my_upsample_ptr;

#define SCALEBITS 16 /* speediest right-shift on some machines */
#define ONE_HALF ((JLONG)1 << (SCALEBITS - 1))
#define FIX(x) ((JLONG)((x) * (1L << SCALEBITS) + 0.5))
Expand Down Expand Up @@ -189,7 +161,7 @@ typedef my_upsampler *my_upsample_ptr;
LOCAL(void)
build_ycc_rgb_table(j_decompress_ptr cinfo)
{
my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
my_merged_upsample_ptr upsample = (my_merged_upsample_ptr)cinfo->upsample;
int i;
JLONG x;
SHIFT_TEMPS
Expand Down Expand Up @@ -232,7 +204,7 @@ build_ycc_rgb_table(j_decompress_ptr cinfo)
METHODDEF(void)
start_pass_merged_upsample(j_decompress_ptr cinfo)
{
my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
my_merged_upsample_ptr upsample = (my_merged_upsample_ptr)cinfo->upsample;

/* Mark the spare buffer empty */
upsample->spare_full = FALSE;
Expand All @@ -254,7 +226,7 @@ merged_2v_upsample(j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
JDIMENSION *out_row_ctr, JDIMENSION out_rows_avail)
/* 2:1 vertical sampling case: may need a spare row. */
{
my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
my_merged_upsample_ptr upsample = (my_merged_upsample_ptr)cinfo->upsample;
JSAMPROW work_ptrs[2];
JDIMENSION num_rows; /* number of rows returned to caller */

Expand Down Expand Up @@ -305,7 +277,7 @@ merged_1v_upsample(j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
JDIMENSION *out_row_ctr, JDIMENSION out_rows_avail)
/* 1:1 vertical sampling case: much easier, never need a spare row. */
{
my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
my_merged_upsample_ptr upsample = (my_merged_upsample_ptr)cinfo->upsample;

/* Just do the upsampling. */
(*upsample->upmethod) (cinfo, input_buf, *in_row_group_ctr,
Expand Down Expand Up @@ -566,11 +538,11 @@ h2v2_merged_upsample_565D(j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
GLOBAL(void)
jinit_merged_upsampler(j_decompress_ptr cinfo)
{
my_upsample_ptr upsample;
my_merged_upsample_ptr upsample;

upsample = (my_upsample_ptr)
upsample = (my_merged_upsample_ptr)
(*cinfo->mem->alloc_small) ((j_common_ptr)cinfo, JPOOL_IMAGE,
sizeof(my_upsampler));
sizeof(my_merged_upsampler));
cinfo->upsample = (struct jpeg_upsampler *)upsample;
upsample->pub.start_pass = start_pass_merged_upsample;
upsample->pub.need_context_rows = FALSE;
Expand Down
47 changes: 47 additions & 0 deletions jdmerge.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* jdmerge.h
*
* This file was part of the Independent JPEG Group's software:
* Copyright (C) 1994-1996, Thomas G. Lane.
* libjpeg-turbo Modifications:
* Copyright (C) 2020, D. R. Commander.
* For conditions of distribution and use, see the accompanying README.ijg
* file.
*/

#define JPEG_INTERNALS
#include "jpeglib.h"

#ifdef UPSAMPLE_MERGING_SUPPORTED


/* Private subobject */

typedef struct {
struct jpeg_upsampler pub; /* public fields */

/* Pointer to routine to do actual upsampling/conversion of one row group */
void (*upmethod) (j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
JDIMENSION in_row_group_ctr, JSAMPARRAY output_buf);

/* Private state for YCC->RGB conversion */
int *Cr_r_tab; /* => table for Cr to R conversion */
int *Cb_b_tab; /* => table for Cb to B conversion */
JLONG *Cr_g_tab; /* => table for Cr to G conversion */
JLONG *Cb_g_tab; /* => table for Cb to G conversion */

/* For 2:1 vertical sampling, we produce two output rows at a time.
* We need a "spare" row buffer to hold the second output row if the
* application provides just a one-row buffer; we also use the spare
* to discard the dummy last row if the image height is odd.
*/
JSAMPROW spare_row;
boolean spare_full; /* T if spare buffer is occupied */

JDIMENSION out_row_width; /* samples per output row */
JDIMENSION rows_to_go; /* counts rows remaining in image */
} my_merged_upsampler;

typedef my_merged_upsampler *my_merged_upsample_ptr;

#endif /* UPSAMPLE_MERGING_SUPPORTED */
10 changes: 5 additions & 5 deletions jdmrg565.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Copyright (C) 1994-1996, Thomas G. Lane.
* libjpeg-turbo Modifications:
* Copyright (C) 2013, Linaro Limited.
* Copyright (C) 2014-2015, 2018, D. R. Commander.
* Copyright (C) 2014-2015, 2018, 2020, D. R. Commander.
* For conditions of distribution and use, see the accompanying README.ijg
* file.
*
Expand All @@ -19,7 +19,7 @@ h2v1_merged_upsample_565_internal(j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
JDIMENSION in_row_group_ctr,
JSAMPARRAY output_buf)
{
my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
my_merged_upsample_ptr upsample = (my_merged_upsample_ptr)cinfo->upsample;
register int y, cred, cgreen, cblue;
int cb, cr;
register JSAMPROW outptr;
Expand Down Expand Up @@ -90,7 +90,7 @@ h2v1_merged_upsample_565D_internal(j_decompress_ptr cinfo,
JDIMENSION in_row_group_ctr,
JSAMPARRAY output_buf)
{
my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
my_merged_upsample_ptr upsample = (my_merged_upsample_ptr)cinfo->upsample;
register int y, cred, cgreen, cblue;
int cb, cr;
register JSAMPROW outptr;
Expand Down Expand Up @@ -163,7 +163,7 @@ h2v2_merged_upsample_565_internal(j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
JDIMENSION in_row_group_ctr,
JSAMPARRAY output_buf)
{
my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
my_merged_upsample_ptr upsample = (my_merged_upsample_ptr)cinfo->upsample;
register int y, cred, cgreen, cblue;
int cb, cr;
register JSAMPROW outptr0, outptr1;
Expand Down Expand Up @@ -259,7 +259,7 @@ h2v2_merged_upsample_565D_internal(j_decompress_ptr cinfo,
JDIMENSION in_row_group_ctr,
JSAMPARRAY output_buf)
{
my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
my_merged_upsample_ptr upsample = (my_merged_upsample_ptr)cinfo->upsample;
register int y, cred, cgreen, cblue;
int cb, cr;
register JSAMPROW outptr0, outptr1;
Expand Down
6 changes: 3 additions & 3 deletions jdmrgext.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This file was part of the Independent JPEG Group's software:
* Copyright (C) 1994-1996, Thomas G. Lane.
* libjpeg-turbo Modifications:
* Copyright (C) 2011, 2015, D. R. Commander.
* Copyright (C) 2011, 2015, 2020, D. R. Commander.
* For conditions of distribution and use, see the accompanying README.ijg
* file.
*
Expand All @@ -25,7 +25,7 @@ h2v1_merged_upsample_internal(j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
JDIMENSION in_row_group_ctr,
JSAMPARRAY output_buf)
{
my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
my_merged_upsample_ptr upsample = (my_merged_upsample_ptr)cinfo->upsample;
register int y, cred, cgreen, cblue;
int cb, cr;
register JSAMPROW outptr;
Expand Down Expand Up @@ -97,7 +97,7 @@ h2v2_merged_upsample_internal(j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
JDIMENSION in_row_group_ctr,
JSAMPARRAY output_buf)
{
my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
my_merged_upsample_ptr upsample = (my_merged_upsample_ptr)cinfo->upsample;
register int y, cred, cgreen, cblue;
int cb, cr;
register JSAMPROW outptr0, outptr1;
Expand Down

0 comments on commit 9120a24

Please sign in to comment.