jcopy_sample_rows() causes SIGSEGV #441

zodf0055980 · 2020-07-06T07:45:09Z

Have you searched the existing issues (both open and closed) in the libjpeg-turbo issue tracker to ensure that this bug report is not a duplicate?
Yes

Does this bug report describe one of the two known and unsolvable issues with the JPEG format?
No

Clear and concise description of the bug:
jcopy_sample_rows try to read *0x8 causes SIGSEGV

0x00007ffff7b7d48a in jcopy_sample_rows (input_array=0x555555764728, source_row=0x0, output_array=0x8, dest_row=0x0, num_rows=0x1, num_cols=0x60) at /home/yuan/libjpeg-turbo/jutils.c:112
112 outptr = *output_array++;

Steps to reproduce the bug (using only libjpeg-turbo):
in read_and_discard_scanlines() , it calls jpeg_read_scanlines(cinfo, NULL, 1);
However NULL will be output array address in jcopy_sample_rows().

Image(s) needed in order to reproduce the bug (if applicable):

Expected behavior:

Observed behavior:

./djpeg -fast -skip 1,20 -outfile ./outt ./sample.jpg

gdb-peda$ bt
#0  0x00007ffff7b7d48a in jcopy_sample_rows (input_array=0x555555764728, source_row=0x0, output_array=0x8, dest_row=0x0, num_rows=0x1, num_cols=0x60) at /home/yuan/libjpeg-turbo/jutils.c:112
#1  0x00007ffff7b61454 in merged_2v_upsample (cinfo=0x7fffffffd970, input_buf=0x555555766610, in_row_group_ctr=0x555555766664, in_row_groups_avail=0x8, output_buf=0x0, out_row_ctr=0x7fffffffd764, 
    out_rows_avail=0x1) at /home/yuan/libjpeg-turbo/jdmerge.c:266
#2  0x00007ffff7b5b5a1 in process_data_simple_main (cinfo=0x7fffffffd970, output_buf=0x0, out_row_ctr=0x7fffffffd764, out_rows_avail=0x1) at /home/yuan/libjpeg-turbo/jdmainct.c:313
#3  0x00007ffff7b4dcc1 in jpeg_read_scanlines (cinfo=0x7fffffffd970, scanlines=0x0, max_lines=0x1) at /home/yuan/libjpeg-turbo/jdapistd.c:285
#4  0x00007ffff7b4ddfb in read_and_discard_scanlines (cinfo=0x7fffffffd970, num_lines=0x1) at /home/yuan/libjpeg-turbo/jdapistd.c:336
#5  0x00007ffff7b4dec9 in increment_simple_rowgroup_ctr (cinfo=0x7fffffffd970, rows=0x5) at /home/yuan/libjpeg-turbo/jdapistd.c:366
#6  0x00007ffff7b4e46d in jpeg_skip_scanlines (cinfo=0x7fffffffd970, num_lines=0x14) at /home/yuan/libjpeg-turbo/jdapistd.c:530
#7  0x0000555555557638 in main (argc=0x7, argv=0x7fffffffdcd8) at /home/yuan/libjpeg-turbo/djpeg.c:711
#8  0x00007ffff7754b97 in __libc_start_main (main=0x555555556f8c <main>, argc=0x7, argv=0x7fffffffdcd8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffdcc8)
    at ../csu/libc-start.c:310
#9  0x0000555555555c6a in _start ()

Platform(s) (compiler version, operating system version, CPU) on which the bug was observed:
gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0 , Linux version 5.3.0-61-generic

libjpeg-turbo release(s), commit(s), or branch(es) in which the bug was observed (always test the tip of the master branch or the latest stable pre-release to verify that the bug hasn't already been fixed):
commit ae87a95 in master

If the bug is a regression, the specific commit that introduced the regression (use git bisect to determine this):

Additional information:

The text was updated successfully, but these errors were encountered:

…jpeg-turbo into 10.0" Newest libjpeg crashes apps on Android. Due to reported bug: libjpeg-turbo/libjpeg-turbo#441 Merge will be reapplies when libjper gets fixed. This reverts commit 6646983, reversing changes made to b99b057. Signed-off-by: xNombre <kartapolska@gmail.com>

…jpeg-turbo into 10.0" Newest libjpeg crashes apps on Android. Due to reported bug: libjpeg-turbo/libjpeg-turbo#441 Merge will be reapplied when libjpeg gets fix for this. Signed-off-by: xNombre <kartapolska@gmail.com>

dcommander · 2020-07-24T04:23:58Z

Should be fixed now. Please test.

zodf0055980 · 2020-07-28T06:05:53Z

It's okay now, thanks!

- Introduce a partial image decompression regression test script that validates the correctness of jpeg_skip_scanlines() and jpeg_crop_scanlines() for a variety of cropping regions and libjpeg settings. This regression test catches the following issues: #182, fixed in 5bc43c7 #237, fixed in 6e95c08 #244, fixed in 398c1e9 #441, fully fixed in this commit It does not catch the following issues: #194, fixed in 773040f #244 (additional segfault), fixed in 9120a24 - Modify the libjpeg-turbo regression test suite (make test) so that it checks for the issue reported in #441 (segfault in jpeg_skip_scanlines() when used with 4:2:0 merged upsampling/color conversion.) - Fix issues in jpeg_skip_scanlines() that caused incorrect output with h2v2 (4:2:0) merged upsampling/color conversion. The previous commit fixed the segfault reported in #441, but that was a symptom of a larger problem. Because merged 4:2:0 upsampling uses a "spare row" buffer, it is necessary to allow the upsampler to run when skipping rows (fancy 4:2:0 upsampling, which uses context rows, also requires this.) Otherwise, if skipping starts at an odd-numbered row, the output image will be incorrect. - Throw an error if jpeg_skip_scanlines() is called with two-pass color quantization enabled. With two-pass color quantization, the first pass occurs within jpeg_start_decompress(), so subsequent calls to jpeg_skip_scanlines() interfere with the multipass state and prevent the second pass from occurring during subsequent calls to jpeg_read_scanlines().

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)

- Introduce a partial image decompression regression test script that validates the correctness of jpeg_skip_scanlines() and jpeg_crop_scanlines() for a variety of cropping regions and libjpeg settings. This regression test catches the following issues: #182, fixed in 5bc43c7 #237, fixed in 6e95c08 #244, fixed in 398c1e9 #441, fully fixed in this commit It does not catch the following issues: #194, fixed in 773040f #244 (additional segfault), fixed in 9120a24 - Modify the libjpeg-turbo regression test suite (make test) so that it checks for the issue reported in #441 (segfault in jpeg_skip_scanlines() when used with 4:2:0 merged upsampling/color conversion.) - Fix issues in jpeg_skip_scanlines() that caused incorrect output with h2v2 (4:2:0) merged upsampling/color conversion. The previous commit fixed the segfault reported in #441, but that was a symptom of a larger problem. Because merged 4:2:0 upsampling uses a "spare row" buffer, it is necessary to allow the upsampler to run when skipping rows (fancy 4:2:0 upsampling, which uses context rows, also requires this.) Otherwise, if skipping starts at an odd-numbered row, the output image will be incorrect. - Throw an error if jpeg_skip_scanlines() is called with two-pass color quantization enabled. With two-pass color quantization, the first pass occurs within jpeg_start_decompress(), so subsequent calls to jpeg_skip_scanlines() interfere with the multipass state and prevent the second pass from occurring during subsequent calls to jpeg_read_scanlines().

(Oversight when cherry-picking ab835bc)

mathias-clerc · 2021-12-23T08:54:45Z

I have a similar issue using v2.1.2.

__memmove_evex_unaligned_erms 0x00007f46229c9383
jcopy_sample_rows 0x0000562324e61427
merged_2v_upsample 0x0000562324e5af50
process_data_simple_main 0x0000562324e6fee9
jpeg_read_scanlines 0x0000562324e5410b

Should I open a separate issue or reopen this one ?

dcommander · 2021-12-23T16:23:45Z

@mathias-clerc This issue was closed and fixed, so open a new issue.

zodf0055980 added the bug label Jul 6, 2020

zodf0055980 assigned dcommander Jul 6, 2020

xNombre mentioned this issue Jul 6, 2020

Revert recent libjpeg upstream due to bugs crdroidandroid/android_external_libjpeg-turbo#2

Merged

zodf0055980 changed the title ~~In jcopy_sample_rows() will cause SIGSEGV~~ jcopy_sample_rows() causes SIGSEGV Jul 13, 2020

dcommander closed this as completed in 9120a24 Jul 24, 2020

dcommander added the fixed label Jul 24, 2020

dcommander reopened this Jul 28, 2020

dcommander added a commit that referenced this issue Jul 28, 2020

Build: Test for issue reported in #441

0208fe0

(Oversight when cherry-picking ab835bc)

dcommander closed this as completed Jul 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

jcopy_sample_rows() causes SIGSEGV #441

jcopy_sample_rows() causes SIGSEGV #441

zodf0055980 commented Jul 6, 2020 •

edited

dcommander commented Jul 24, 2020

zodf0055980 commented Jul 28, 2020

mathias-clerc commented Dec 23, 2021

dcommander commented Dec 23, 2021

jcopy_sample_rows() causes SIGSEGV #441

jcopy_sample_rows() causes SIGSEGV #441

Comments

zodf0055980 commented Jul 6, 2020 • edited

dcommander commented Jul 24, 2020

zodf0055980 commented Jul 28, 2020

mathias-clerc commented Dec 23, 2021

dcommander commented Dec 23, 2021

zodf0055980 commented Jul 6, 2020 •

edited