New issue
Advanced search Search tips

Issue 846800 link

Starred by 3 users
Status: Fixed
Owner:
tsepez@chromium.org
Closed: May 2018
Cc:
rsesek@chromium.org
tsepez@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug

Blocked on:
issue 847096
Blocking:
issue 846702



Sign in to add a comment

Linux network service sandbox breaks base::DeleteFile on cache paths.

Project Member Reported by morlovich@chromium.org, May 25 2018 
To reproduce:
1) chrome --enable-features=NetworkService on Linux
2) Go visit some webpages, e.g. www.cnn.com
3) Look at ~/.cache/chromium/Default/Cache and see it full of todelete_ files

The code actually calls base::DeleteFile on all these paths. base::DeleteFile calls some sort of lstat variant here:
https://cs.chromium.org/chromium/src/base/files/file_util_posix.cc?rcl=de31df46c99603235f83bbcdc7c8af1fbd62b5ff&l=382

gets ENOENT, and decides there is nothing more to do.

The problem goes away by disabling the network process sandbox via:
--- a/services/service_manager/sandbox/sandbox_type.cc
+++ b/services/service_manager/sandbox/sandbox_type.cc
@@ -16,6 +16,9 @@
 namespace service_manager {
 
 bool IsUnsandboxedSandboxType(SandboxType sandbox_type) {
+  if (sandbox_type == SANDBOX_TYPE_NETWORK)
+    return true;



(This is the proximate cause of  bug 846702 )

Comment 1 by morlovich@chromium.org, May 25 2018

Blocking: 846702

Comment 2 by morlovich@chromium.org, May 25 2018

Labels: Proj-Servicification

Comment 3 by tsepez@chromium.org, May 25 2018

Cc: rsesek@chromium.org
Owner: tsepez@chromium.org
Status: Assigned (was: Untriaged)
Looks like we're proxying stat() and fstat() variants, but not lstat().

Comment 5 by morlovich@chromium.org, May 25 2018 

It does appear to fix things. Thanks!
Project Member

Comment 6 by bugdroid1@chromium.org, May 25 2018 

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

commit 4b0e0023a1190ff6534b514c00b7e9c1d611bb5c
Author: Tom Sepez <tsepez@chromium.org>
Date: Fri May 25 22:55:09 2018

Proxy lstat() system calls to broker process for network service.

This particular stat() variant was not being handled in the same
way as the others.

Bug:  846800 
Change-Id: I2ec9b94f704f9f00aa3dd1dbd0a3be71a0583b75
Reviewed-on: https://chromium-review.googlesource.com/1073869
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562045}
[modify] https://crrev.com/4b0e0023a1190ff6534b514c00b7e9c1d611bb5c/sandbox/linux/syscall_broker/broker_client.cc
[modify] https://crrev.com/4b0e0023a1190ff6534b514c00b7e9c1d611bb5c/sandbox/linux/syscall_broker/broker_client.h
[modify] https://crrev.com/4b0e0023a1190ff6534b514c00b7e9c1d611bb5c/sandbox/linux/syscall_broker/broker_host.cc
[modify] https://crrev.com/4b0e0023a1190ff6534b514c00b7e9c1d611bb5c/sandbox/linux/syscall_broker/broker_process.cc
[modify] https://crrev.com/4b0e0023a1190ff6534b514c00b7e9c1d611bb5c/sandbox/linux/syscall_broker/broker_process.h
[modify] https://crrev.com/4b0e0023a1190ff6534b514c00b7e9c1d611bb5c/sandbox/linux/syscall_broker/broker_process_unittest.cc
[modify] https://crrev.com/4b0e0023a1190ff6534b514c00b7e9c1d611bb5c/services/service_manager/sandbox/linux/bpf_broker_policy_linux.cc
[modify] https://crrev.com/4b0e0023a1190ff6534b514c00b7e9c1d611bb5c/services/service_manager/sandbox/linux/bpf_network_policy_linux.cc

Comment 7 by tsepez@chromium.org, May 25 2018

Status: Fixed (was: Assigned)
Project Member

Comment 8 by bugdroid1@chromium.org, May 28 2018 

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

commit 58a387f3e083504564fbe31b583f1fd9cd4f8964
Author: Trent Apted <tapted@chromium.org>
Date: Mon May 28 01:53:26 2018

Revert "Proxy lstat() system calls to broker process for network service."

This reverts commit 4b0e0023a1190ff6534b514c00b7e9c1d611bb5c.

Reason for revert: Suspect for uninitialized memory access

==8094==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x1b90a45f in sandbox::syscall_broker::BrokerFilePermission::ValidatePath(char const*)

and failing NetworkContextConfigurationBrowserTest.FileURL tests since

https://ci.chromium.org/buildbot/chromium.memory/Linux%20MSan%20Tests/10019

Original change's description:
> Proxy lstat() system calls to broker process for network service.
>
> This particular stat() variant was not being handled in the same
> way as the others.
>
> Bug:  846800 
> Change-Id: I2ec9b94f704f9f00aa3dd1dbd0a3be71a0583b75
> Reviewed-on: https://chromium-review.googlesource.com/1073869
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Commit-Queue: Tom Sepez <tsepez@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#562045}

TBR=tsepez@chromium.org,rsesek@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  846800 ,  847096 
Change-Id: I49b936a9afc0491fb9217c165c2060b760d7b0ba
Reviewed-on: https://chromium-review.googlesource.com/1074847
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562159}
[modify] https://crrev.com/58a387f3e083504564fbe31b583f1fd9cd4f8964/sandbox/linux/syscall_broker/broker_client.cc
[modify] https://crrev.com/58a387f3e083504564fbe31b583f1fd9cd4f8964/sandbox/linux/syscall_broker/broker_client.h
[modify] https://crrev.com/58a387f3e083504564fbe31b583f1fd9cd4f8964/sandbox/linux/syscall_broker/broker_host.cc
[modify] https://crrev.com/58a387f3e083504564fbe31b583f1fd9cd4f8964/sandbox/linux/syscall_broker/broker_process.cc
[modify] https://crrev.com/58a387f3e083504564fbe31b583f1fd9cd4f8964/sandbox/linux/syscall_broker/broker_process.h
[modify] https://crrev.com/58a387f3e083504564fbe31b583f1fd9cd4f8964/sandbox/linux/syscall_broker/broker_process_unittest.cc
[modify] https://crrev.com/58a387f3e083504564fbe31b583f1fd9cd4f8964/services/service_manager/sandbox/linux/bpf_broker_policy_linux.cc
[modify] https://crrev.com/58a387f3e083504564fbe31b583f1fd9cd4f8964/services/service_manager/sandbox/linux/bpf_network_policy_linux.cc

Comment 9 by tsepez@chromium.org, May 29 2018

Status: Assigned (was: Fixed)

Comment 10 by tsepez@chromium.org, May 29 2018

Blockedon: 847096

Comment 11 by dxie@chromium.org, May 29 2018

Labels: Proj-Servicification-Canary Hotlist-KnownIssue OS-Linux
Project Member

Comment 12 by bugdroid1@chromium.org, May 30 2018 

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

commit c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90
Author: Tom Sepez <tsepez@chromium.org>
Date: Wed May 30 17:29:14 2018

Reland "Proxy lstat() system calls to broker process for network service."

This reverts commit 58a387f3e083504564fbe31b583f1fd9cd4f8964.

The original patch was reverted due to a spurious msan warning. This
version unpoisons the buffer per discussion on  bug 847096 .

Bug:  847096 , 846800 
Change-Id: Ibd82c5c6d46521325ff81146a8f000deff05148c
Reviewed-on: https://chromium-review.googlesource.com/1077432
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562889}
[modify] https://crrev.com/c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90/sandbox/linux/syscall_broker/broker_client.cc
[modify] https://crrev.com/c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90/sandbox/linux/syscall_broker/broker_client.h
[modify] https://crrev.com/c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90/sandbox/linux/syscall_broker/broker_host.cc
[modify] https://crrev.com/c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90/sandbox/linux/syscall_broker/broker_process.cc
[modify] https://crrev.com/c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90/sandbox/linux/syscall_broker/broker_process.h
[modify] https://crrev.com/c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90/sandbox/linux/syscall_broker/broker_process_unittest.cc
[modify] https://crrev.com/c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90/services/service_manager/sandbox/linux/bpf_broker_policy_linux.cc
[modify] https://crrev.com/c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90/services/service_manager/sandbox/linux/bpf_network_policy_linux.cc

Comment 13 by tsepez@chromium.org, May 30 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment