Skip to content

GitLab

T
tangled
Commit ec87aec2 authored by Ojan Vafai's avatar Ojan Vafai Committed by Commit Bot
Browse files
Options

Revert "Enable the seccomp-bpf sandbox for the Android photo picker" 

This reverts commit 83622231.

Reason for revert: Caused test failures on https://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%29/builds/44157

org.chromium.chrome.browser.photo_picker.PhotoPickerDialogTest#testSingleSelectionPhoto
org.chromium.chrome.browser.photo_picker.PhotoPickerDialogTest#testMultiSelectionPhoto
org.chromium.chrome.browser.photo_picker.PhotoPickerDialogTest#testNoSelection

Original change's description:
> Enable the seccomp-bpf sandbox for the Android photo picker
> 
> This CL attempts to enable the seccomp-bpf sandbox, as we do for
> renderer processes on Android, for the new photo picker when it is
> supported by the device.
> 
> The implementation loads the native library as if it's a Chrome child
> process, and then calls the native InitializePhotoPickerSandbox function
> to initialize the sandbox when available. UMA is logged.
> 
> Because we end up loading the native library, this does create for an
> additional delay of ~700ms before the first photo is shown. There may be
> optimization opportunities by separating out the sandbox code in a separate
> shared library, but this is made difficult by (a) our build system using
> the crazy linker, and (b) the dependencies of //sandbox on //base.
> 
> I've verified that the sandbox is enabled on two devices running
> different versions of Android, both by printing the status of the
> SeccompStarterAndroid, and through the following command:
> 
> $ adb shell cat /proc/18424/status | grep Seccomp
> Seccomp: 2
> 
> (Where "2" means that seccomp-bpf is enabled.)
> 
> BUG=730066
> 
> Change-Id: I46e608bad8f69d3cf862c0953361c50f4c65c45c
> Reviewed-on: https://chromium-review.googlesource.com/577853
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
> Reviewed-by: Bo Liu <boliu@chromium.org>
> Reviewed-by: Ilya Sherman <isherman@chromium.org>
> Commit-Queue: Peter Beverloo <peter@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#488643}

TBR=jorgelo@chromium.org,peter@chromium.org,isherman@chromium.org,boliu@chromium.org,dtrainor@chromium.org,twellington@chromium.org

Change-Id: I2b0d24e90ce50bf2bc913cc7219f48475a34bbda
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 730066
Reviewed-on: https://chromium-review.googlesource.com/581628Reviewed-by: default avatarOjan Vafai <ojan@chromium.org>
Commit-Queue: Ojan Vafai <ojan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488686}
parent e24f4166
Hide whitespace changes
Inline Side-by-side
Showing with 3 additions and 73 deletions
chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderService.java
View file @ ec87aec2
......@@ -14,9 +14,6 @@ import android.os.RemoteException;
import android.os.SystemClock;
import org.chromium.base.Log;
import org.chromium.base.library_loader.LibraryLoader;
import org.chromium.base.library_loader.LibraryProcessType;
import org.chromium.base.library_loader.ProcessInitException;
import java.io.FileDescriptor;
import java.io.IOException;
......@@ -36,20 +33,8 @@ public class DecoderService extends Service {
// A tag for logging error messages.
private static final String TAG = "ImageDecoder";
// Whether the native library and the sandbox have been initialized.
private boolean mNativeLibraryAndSandboxInitialized;
@Override
public void onCreate() {
try {
LibraryLoader.get(LibraryProcessType.PROCESS_CHILD).ensureInitialized();
nativeInitializePhotoPickerSandbox();
mNativeLibraryAndSandboxInitialized = true;
} catch (ProcessInitException e) {
Log.e(TAG, "Unable to initialize the native library and sandbox", e);
}
super.onCreate();
}
......@@ -74,12 +59,6 @@ public class DecoderService extends Service {
bundle.putString(KEY_FILE_PATH, filePath);
bundle.putBoolean(KEY_SUCCESS, false);
if (!mNativeLibraryAndSandboxInitialized) {
Log.e(TAG, "Decode failed %s (size: %d): no sandbox", filePath, size);
sendReply(callback, bundle); // Sends SUCCESS == false;
return;
}
FileDescriptor fd = pfd.getFileDescriptor();
long begin = SystemClock.elapsedRealtime();
......@@ -128,8 +107,4 @@ public class DecoderService extends Service {
}
}
};
// Initializes the seccomp-bpf sandbox when it's supported by the device. Records the sandbox
// status to the Android.SeccompStatus.PhotoPickerSandbox histogram.
private static native void nativeInitializePhotoPickerSandbox();
}
chrome/browser/BUILD.gn
View file @ ec87aec2
......@@ -2844,7 +2844,6 @@ split_static_library("browser") {
"android/password_ui_view_android.cc",
"android/password_ui_view_android.h",
"android/payments/service_worker_payment_app_bridge.cc",
"android/photo_picker_sandbox_bridge.cc",
"android/physical_web/eddystone_encoder_bridge.cc",
"android/physical_web/eddystone_encoder_bridge.h",
"android/physical_web/physical_web_data_source_android.cc",
......@@ -3078,7 +3077,6 @@ split_static_library("browser") {
"//components/toolbar",
"//components/web_contents_delegate_android",
"//rlz:rlz_utils",
"//sandbox",
"//sandbox:sandbox_features",
"//third_party/android_opengl/etc1",
"//third_party/android_tools:cpu_features",
......@@ -3088,6 +3086,9 @@ split_static_library("browser") {
]
deps -= [ "//components/storage_monitor" ]
if (use_seccomp_bpf) {
deps += [ "//sandbox/linux:seccomp_bpf" ]
}
if (enable_supervised_users) {
sources += [
......@@ -4112,7 +4113,6 @@ if (is_android) {
"../android/java/src/org/chromium/chrome/browser/payments/SslValidityChecker.java",
"../android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogController.java",
"../android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogDelegate.java",
"../android/java/src/org/chromium/chrome/browser/photo_picker/DecoderService.java",
"../android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebBroadcastService.java",
"../android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java",
"../android/java/src/org/chromium/chrome/browser/policy/PolicyAuditor.java",
......
chrome/browser/android/DEPS
View file @ ec87aec2
......@@ -12,7 +12,6 @@ include_rules = [
"+components/web_contents_delegate_android",
"+device/vr/features/features.h",
"+sandbox/linux/seccomp-bpf/sandbox_bpf.h",
"+sandbox/linux/seccomp-bpf-helpers",
"+sandbox/sandbox_features.h",
"+third_party/gvr-android-sdk",
]
......
chrome/browser/android/photo_picker_sandbox_bridge.cc deleted 100644 → 0
View file @ e24f4166
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "base/android/build_info.h"
#include "base/android/jni_android.h"
#include "base/android/scoped_java_ref.h"
#include "base/metrics/histogram_macros.h"
#include "jni/DecoderService_jni.h"
#include "sandbox/linux/seccomp-bpf-helpers/seccomp_starter_android.h"
#include "sandbox/sandbox_features.h"
#if BUILDFLAG(USE_SECCOMP_BPF)
#include "base/memory/ptr_util.h"
#include "sandbox/linux/seccomp-bpf-helpers/baseline_policy_android.h"
#endif
void InitializePhotoPickerSandbox(
JNIEnv* env,
const base::android::JavaParamRef<jclass>& jcaller) {
auto* info = base::android::BuildInfo::GetInstance();
sandbox::SeccompStarterAndroid starter(info->sdk_int(), info->device());
#if BUILDFLAG(USE_SECCOMP_BPF)
// The policy compiler is only available if USE_SECCOMP_BPF is enabled.
starter.set_policy(base::MakeUnique<sandbox::BaselinePolicyAndroid>());
#endif
starter.StartSandbox();
UMA_HISTOGRAM_ENUMERATION("Android.SeccompStatus.PhotoPickerSandbox",
starter.status(),
sandbox::SeccompSandboxStatus::STATUS_MAX);
}
tools/metrics/histograms/histograms.xml
View file @ ec87aec2
......@@ -1126,17 +1126,6 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
</summary>
</histogram>
<histogram name="Android.SeccompStatus.PhotoPickerSandbox"
enum="AndroidSeccompSandboxStatus">
<owner>peter@chromium.org</owner>
<owner>rsesek@chromium.org</owner>
<summary>
Reports the status of the seccomp-bpf sandbox in photo picker decoding
processes. Anything other than &quot;Sandbox Engaged&quot; indicates the
sandbox is not turned on. See https://crbug.com/477049.
</summary>
</histogram>
<histogram name="Android.SeccompStatus.Prctl" enum="AndroidSeccompStatus">
<owner>rsesek@chromium.org</owner>
<summary>
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment