New issue
Advanced search Search tips

Issue 776905 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Fix individual function pointer casts causing requiring cfi-icall blacklist entries

Project Member Reported by vtsyrklevich@chromium.org, Oct 20 2017

Issue description

Great
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 20 2017

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

commit 52e618094ac1f38a02288ca585a62672dfb4fd9f
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Oct 20 21:25:23 2017

[cfi-icall] Refactor SetTestingFactory methods

Control Flow Integrity indirect call checking (cfi-icall) requires that
function pointers be called with the type signature matching the called
function. Callers of (Refcounted)KeyedServiceFactory::SetTestingFactory
cast the TestingFactoryFunction function pointer to match what
(Refcounted)KeyedServiceFactory expects. Refactor that interface to
accept a std::function<> that calls the function pointers with the
correct type signatures instead.

SetTestingFactory()/SetTestingFactoryAndUse() can accept NULL function
pointers, maintain those semantics by passing an empty std::function if
a NULL TestingFactoryFunction was provided.

Bug: 776905
Change-Id: I66ea1e126792268c1e4645574b69ce0ab73ee758
Reviewed-on: https://chromium-review.googlesource.com/731508
Reviewed-by: Elliot Glaysher <erg@chromium.org>
Reviewed-by: Peter Collingbourne <pcc@chromium.org>
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510565}
[modify] https://crrev.com/52e618094ac1f38a02288ca585a62672dfb4fd9f/components/keyed_service/content/browser_context_keyed_service_factory.cc
[modify] https://crrev.com/52e618094ac1f38a02288ca585a62672dfb4fd9f/components/keyed_service/content/refcounted_browser_context_keyed_service_factory.cc
[modify] https://crrev.com/52e618094ac1f38a02288ca585a62672dfb4fd9f/components/keyed_service/core/keyed_service_factory.h
[modify] https://crrev.com/52e618094ac1f38a02288ca585a62672dfb4fd9f/components/keyed_service/core/refcounted_keyed_service_factory.h
[modify] https://crrev.com/52e618094ac1f38a02288ca585a62672dfb4fd9f/components/keyed_service/ios/browser_state_keyed_service_factory.cc
[modify] https://crrev.com/52e618094ac1f38a02288ca585a62672dfb4fd9f/components/keyed_service/ios/refcounted_browser_state_keyed_service_factory.cc
[modify] https://crrev.com/52e618094ac1f38a02288ca585a62672dfb4fd9f/tools/cfi/blacklist.txt

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 6 2017

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

commit a743b2c97ec0e385ec83d54bb344f0c8aec6e529
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Wed Dec 06 16:56:39 2017

[CFI] Make base/once implementation CFI-icall safe

Control Flow Integrity [1] indirect call checking is a compiler-
instrumentation that verifies that function pointers are only used to
call functions whose type signatures matches the type of the function
pointer. The once implementation casts function pointers to different
types--refactor it to pass around a std::function that calls the
provided function pointer with its original type instead.

[1] https://www.chromium.org/developers/testing/control-flow-integrity

Bug=chromium:776905, v8:7164 

Change-Id: I26b63973fdcebbc11b1cd7b023a6a6d0724c360c
Reviewed-on: https://chromium-review.googlesource.com/806262
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49906}
[modify] https://crrev.com/a743b2c97ec0e385ec83d54bb344f0c8aec6e529/src/base/once.cc
[modify] https://crrev.com/a743b2c97ec0e385ec83d54bb344f0c8aec6e529/src/base/once.h

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 12 2017

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

commit e8e8ad8d19ce1bed8200c2e5bc0c2cb9945a8b87
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Tue Dec 12 09:27:07 2017

[CFI] Fix cfi-icall failures

Control Flow Integrity [1] indirect call checking verifies that function
pointers only call valid functions with a matching type signature.
webrtc casts the function pointers for external_hmac causing a cfi-icall
failure when they are later called in libsrtp. Refactor the functions to
match the correct type signatures to avoid this failure.

[1] https://www.chromium.org/developers/testing/control-flow-integrity

Bug: chromium:776905
Change-Id: I419028be02e6c151c497e3ec64f10f35e07cdb0f
Reviewed-on: https://webrtc-review.googlesource.com/26721
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21220}
[modify] https://crrev.com/e8e8ad8d19ce1bed8200c2e5bc0c2cb9945a8b87/pc/externalhmac.cc
[modify] https://crrev.com/e8e8ad8d19ce1bed8200c2e5bc0c2cb9945a8b87/pc/externalhmac.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/webm/libvpx/+/1633786bfb5a3566d55ccdf827dd7999b2c2fab4

commit 1633786bfb5a3566d55ccdf827dd7999b2c2fab4
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Thu Jan 11 00:38:26 2018

[CFI] Remove function pointer casts

Control Flow Integrity [1] indirect call checking verifies that function
pointers only call valid functions with a matching type signature. This
change eliminates some function pointer casts that I missed in my last
CL https://crrev.com/c/780144.

BUG=chromium:776905

[1] https://www.chromium.org/developers/testing/control-flow-integrity

Change-Id: I1c7adbdfffa4fe0b62e993bfb31d06e64b022d66

[modify] https://crrev.com/1633786bfb5a3566d55ccdf827dd7999b2c2fab4/vp9/encoder/vp9_ethread.c

Sign in to add a comment