New issue
Advanced search Search tips

Issue 866290 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jul 31
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug


Show other hotlists

Hotlists containing this issue:
Chromium-Packagers


Sign in to add a comment

JPEG image crashes tab when Chromium is built with -fsanitize=cfi-icall and unbundled libjpeg

Reported by evange...@foutrelis.com, Jul 22

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.59 Safari/537.36

Steps to reproduce the problem:
1. Build Chromium 68 with these arguments:

     use_system_libjpeg=true
     is_cfi=true
     use_cfi_icall=true

   (tested with libjpeg-turbo 1.5.3)

2. Visit https://foutrelis.com/cfi-icall-aw-snap.jpg

What is the expected behavior?

What went wrong?
Tab crashes immediately with "Aw, Snap!".

Crashed report ID: (Not available in Chromium builds.)

How much crashed? Just one tab

Is it a problem with a plugin? No 

Did this work before? N/A 

Chrome version: 68.0.3440.59  Channel: n/a
OS Version: 
Flash Version: 

Just to be clear, this doesn't affect Chrome; only Chromium builds with unbundled libjpeg are affected.

The crash appears to be caused by a CFI violation of an indirect call:

    third_party/blink/renderer/platform/image decoders/jpeg/jpeg_image_decoder.cc:655:14: runtime error: control flow integrity check for type 'unsigned char **(jpeg_common_struct *, int, unsigned int, unsigned int)' failed during indirect function call
(/usr/lib/libjpeg.so.8+0x2f2f0): note: (unknown) defined here

The code at jpeg_image_decoder.cc:655 is this part in AllocateSampleArray():

    if (info_.out_color_space != JCS_YCbCr)
      return (*info_.mem->alloc_sarray)(
          reinterpret_cast_ptr<j_common_ptr>(&info_), JPOOL_IMAGE,
          4 * info_.output_width, 1);

https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/image-decoders/jpeg/jpeg_image_decoder.cc?l=655&rcl=c0419ddbcbef45e6966ac1dbf28663bd68c8f1b0
 
Cc: kkaluri@chromium.org
Components: Build
Labels: Needs-Feedback
evangelos@ For further triage could you please help us with 16 digit crash id from chrome://crashes
Crash.png
15.1 KB View Download
It's an unofficial Chromium build so there's no crash ID. Chrome itself is fine and doesn't crash on the example JPEG.

This issue is mainly a request for help in figuring out if use_cfi_icall=true can be enabled in Linux distro builds of Chromium with unbundled libjpeg; the relevant people should have been pinged through:  https://crbug.com/701919#c18 .
Project Member

Comment 3 by sheriffbot@chromium.org, Jul 23

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: p...@chromium.org
Owner: vtsyrklevich@chromium.org
Labels: TE-NeedsTraige-help
Seems it is out of scope from TE end as it is build related issue, adding TE-NeedsTraige-help label to move this out of our triaging bucket.

Thanks..!
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/db82db1b609f30d144d45477f55697818bcd363c

commit db82db1b609f30d144d45477f55697818bcd363c
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Tue Jul 31 01:03:22 2018

Fix cfi-icall failure with use_system_libjpeg=true

JPEGImageReader::AllocateSampleArray() can call the function pointer
(*info_.mem->alloc_sarray) which can be set by the systems non-CFI
enabled libjpeg DSO when chromium is built with use_system_libjpeg=true.
Disable cfi-icall for that method.

Bug:  866290 
Change-Id: I6d9bbf08c514d6d5f48ad34c3802c63419ed1223
Reviewed-on: https://chromium-review.googlesource.com/1155927
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579270}
[modify] https://crrev.com/db82db1b609f30d144d45477f55697818bcd363c/third_party/blink/renderer/platform/image-decoders/jpeg/jpeg_image_decoder.cc
[modify] https://crrev.com/db82db1b609f30d144d45477f55697818bcd363c/third_party/blink/renderer/platform/wtf/compiler.h

Could you confirm that the above change fixes your issue?
It does fix it! Thanks Vlad.
Random thought: Would it perhaps be preferable to keep cfi-icall enabled for AllocateSampleArray() in builds with bundled libjpeg (by wrapping NO_SANITIZE_CFI_ICALL in "#if defined(USE_SYSTEM_LIBJPEG)"), or does it not matter in this case?
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/20f81a066ffdf6bd30fb4b696b8b3e101368e2f6

commit 20f81a066ffdf6bd30fb4b696b8b3e101368e2f6
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Tue Jul 31 23:21:09 2018

Only disable cfi-icall when use_system_libjpeg=true

Bug:  866290 
Change-Id: Ic5d175b3b854665f50781650406d599d09ee9849
Reviewed-on: https://chromium-review.googlesource.com/1157136
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579614}
[modify] https://crrev.com/20f81a066ffdf6bd30fb4b696b8b3e101368e2f6/third_party/blink/renderer/platform/image-decoders/jpeg/jpeg_image_decoder.cc

Status: Verified (was: Unconfirmed)
Thanks for bringing that up, I originally thought that code path was only reachable with use_system_libjpeg=true but that doesn't appear to be true and #ifdef'ing out that attribute is more hygienic either way.

Sign in to add a comment