Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
494743003:
sandbox: Add support for the new seccomp() system call in kernel 3.17. (Closed)
|
Created:
6 years, 4 months ago by Robert Sesek Modified:
6 years, 3 months ago Reviewers:
jln (very slow on Chromium) CC:
chromium-reviews, jln+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Project:
chromium Visibility:
Public. |
Descriptionsandbox: Add support for the new seccomp() system call in kernel 3.17.
This new system call can be used to set NO_NEW_PRIVS, install a BPF filter,
and synchronize that filter across the thread group.
BUG=308763, 308775
TEST=New SandboxBPF.Tsync test where kernel support is available.
R=jln@chromium.org
Committed: https://crrev.com/07884070ecfe2e6ad00a5493dfdc4df891bbd142
Cr-Commit-Position: refs/heads/master@{#291739}
Patch Set 1 #
Total comments: 29
Patch Set 2 : Address comments #
Total comments: 4
Patch Set 3 : Assert thread state #
Total comments: 11
Patch Set 4 : Always take seccomp() path #Patch Set 5 : Rebase #
Total comments: 2
Patch Set 6 : Put on tinfoil hat #Patch Set 7 : Fix clang compile #
Messages
Total messages: 17 (0 generated)
Look good, with mostly minor remarks! https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/seccomp... File sandbox/linux/seccomp-bpf/sandbox_bpf.cc (right): https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/seccomp... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:441: int rv = syscall(__NR_seccomp); Do you want to add a comment saying that synchronization and the syscall have been added together? https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/seccomp... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:445: return STATUS_UNKNOWN; Maybe add a NOTREACHED() here? https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/seccomp... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:451: return STATUS_UNSUPPORTED; DCHECK(ENOSYS == errno) maybe? https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/seccomp... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:474: if (proc_fd_ < 0) { Ugh I need to get rid of this(always pass a file descriptor). https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/seccomp... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:481: thread_state = PROCESS_MULTI_THREADED; I would still want us to crash if the caller passes PROCESS_SINGLE_THREADED from a multi-threaded process. It may be best to save the result of SupportsSeccompThreadFilterSynchronization() in a auxiliary const bool instead. Then decide whether to call the old or new interface based on that bool. https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/seccomp... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:555: // Install BPF filter program. If the thread state indicates multi-threading Should we still try to enable NO_NEW_PRIV via prctl first? This granularity could be useful in understanding failures (also it makes the code below a bit less nested). https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/seccomp... File sandbox/linux/seccomp-bpf/sandbox_bpf.h (right): https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/seccomp... sandbox/linux/seccomp-bpf/sandbox_bpf.h:100: static SandboxStatus SupportsSeccompThreadFilterSynchronization(); I think I would rather have this and SupportsSeccompSandbox() merged and return a better status. WDYT? No need to do it in this CL, there is a lot of stuff from the early days that I want to remove / simplify here anyways. https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/seccomp... File sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc (right): https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/seccomp... sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2159: // Nanosleep is now blacklisted, so this should fail. I would split that into a separate NanoSleepFails() function. You can call it on the main thread (before the policy is applied) and check that it's false (to test the function itself) and re-use it here and check that it returns true. It's a detail, so up to you. https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/seccomp... sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2168: TEST(SandboxBPF, Tsync) { We should not write tests that affects the current process (which can then run other tests). Please, make this a SANDBOX_TEST instead (sandbox/linux/tests/unit_tests.h). Also using BPF_ASSERT would terminate the whole test suite if you're not inside a SANDBOX_TEST. https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/seccomp... sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2193: If you feel like writing more tests, I think a death test checking that we crash when passing PROCESS_SINGLE_THREADED on a multithreaded process could be useful. https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/service... File sandbox/linux/services/arm_linux_syscalls.h (right): https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/service... sandbox/linux/services/arm_linux_syscalls.h:1349: Do you mind adding the other syscalls that we're missing here? Or at least document explicitly that there is a hole here. https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/service... File sandbox/linux/services/mips_linux_syscalls.h (right): https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/service... sandbox/linux/services/mips_linux_syscalls.h:1411: Same remark: please fill or document hole. https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/service... File sandbox/linux/services/x86_32_linux_syscalls.h (right): https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/service... sandbox/linux/services/x86_32_linux_syscalls.h:1396: Same remark: please fill or document hole. https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/service... File sandbox/linux/services/x86_64_linux_syscalls.h (right): https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/service... sandbox/linux/services/x86_64_linux_syscalls.h:1264: Same remark: please fill or document hole.
Thanks for the review! https://codereview.chromium.org/494743003/diff/1/sandbox/linux/seccomp-bpf/sa... File sandbox/linux/seccomp-bpf/sandbox_bpf.cc (right): https://codereview.chromium.org/494743003/diff/1/sandbox/linux/seccomp-bpf/sa... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:441: int rv = syscall(__NR_seccomp); On 2014/08/20 21:34:20, jln wrote: > Do you want to add a comment saying that synchronization and the syscall have > been added together? Done. https://codereview.chromium.org/494743003/diff/1/sandbox/linux/seccomp-bpf/sa... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:445: return STATUS_UNKNOWN; On 2014/08/20 21:34:20, jln wrote: > Maybe add a NOTREACHED() here? Done. https://codereview.chromium.org/494743003/diff/1/sandbox/linux/seccomp-bpf/sa... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:451: return STATUS_UNSUPPORTED; On 2014/08/20 21:34:20, jln wrote: > DCHECK(ENOSYS == errno) maybe? Done. https://codereview.chromium.org/494743003/diff/1/sandbox/linux/seccomp-bpf/sa... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:474: if (proc_fd_ < 0) { On 2014/08/20 21:34:20, jln wrote: > Ugh I need to get rid of this(always pass a file descriptor). Acknowledged. https://codereview.chromium.org/494743003/diff/1/sandbox/linux/seccomp-bpf/sa... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:481: thread_state = PROCESS_MULTI_THREADED; On 2014/08/20 21:34:20, jln wrote: > I would still want us to crash if the caller passes PROCESS_SINGLE_THREADED from > a multi-threaded process. > > It may be best to save the result of > SupportsSeccompThreadFilterSynchronization() in a auxiliary const bool instead. > > Then decide whether to call the old or new interface based on that bool. I actually think SandboxThreadState should just go away, since we have an easy way to test whether or not a system supports tsync. Back when we thought the interface was going to be prctl(), this wasn't the case. https://codereview.chromium.org/494743003/diff/1/sandbox/linux/seccomp-bpf/sa... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:555: // Install BPF filter program. If the thread state indicates multi-threading On 2014/08/20 21:34:20, jln wrote: > Should we still try to enable NO_NEW_PRIV via prctl first? This granularity > could be useful in understanding failures (also it makes the code below a bit > less nested). Done. https://codereview.chromium.org/494743003/diff/1/sandbox/linux/seccomp-bpf/sa... File sandbox/linux/seccomp-bpf/sandbox_bpf.h (right): https://codereview.chromium.org/494743003/diff/1/sandbox/linux/seccomp-bpf/sa... sandbox/linux/seccomp-bpf/sandbox_bpf.h:100: static SandboxStatus SupportsSeccompThreadFilterSynchronization(); On 2014/08/20 21:34:20, jln wrote: > I think I would rather have this and SupportsSeccompSandbox() merged and return > a better status. WDYT? I agree that SupportsSeccompSandbox should do this, but I do want this separate method for use in tests, too. > No need to do it in this CL, there is a lot of stuff from the early days that I > want to remove / simplify here anyways. Yes, this code can probably be cleaned up a lot now that we have sys_seccomp(). https://codereview.chromium.org/494743003/diff/1/sandbox/linux/seccomp-bpf/sa... File sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc (right): https://codereview.chromium.org/494743003/diff/1/sandbox/linux/seccomp-bpf/sa... sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2159: // Nanosleep is now blacklisted, so this should fail. On 2014/08/20 21:34:20, jln wrote: > I would split that into a separate NanoSleepFails() function. You can call it on > the main thread (before the policy is applied) and check that it's false (to > test the function itself) and re-use it here and check that it returns true. > > It's a detail, so up to you. Done. https://codereview.chromium.org/494743003/diff/1/sandbox/linux/seccomp-bpf/sa... sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2168: TEST(SandboxBPF, Tsync) { On 2014/08/20 21:34:21, jln wrote: > We should not write tests that affects the current process (which can then run > other tests). Please, make this a SANDBOX_TEST instead > (sandbox/linux/tests/unit_tests.h). > > Also using BPF_ASSERT would terminate the whole test suite if you're not inside > a SANDBOX_TEST. Done. https://codereview.chromium.org/494743003/diff/1/sandbox/linux/seccomp-bpf/sa... sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2193: On 2014/08/20 21:34:20, jln wrote: > If you feel like writing more tests, I think a death test checking that we crash > when passing PROCESS_SINGLE_THREADED on a multithreaded process could be useful. Depending on the discussion around the other comment regarding SandboxThreadState, I'll do this. https://codereview.chromium.org/494743003/diff/1/sandbox/linux/services/arm_l... File sandbox/linux/services/arm_linux_syscalls.h (right): https://codereview.chromium.org/494743003/diff/1/sandbox/linux/services/arm_l... sandbox/linux/services/arm_linux_syscalls.h:1349: On 2014/08/20 21:34:21, jln wrote: > Do you mind adding the other syscalls that we're missing here? > > Or at least document explicitly that there is a hole here. Done. https://codereview.chromium.org/494743003/diff/1/sandbox/linux/services/mips_... File sandbox/linux/services/mips_linux_syscalls.h (right): https://codereview.chromium.org/494743003/diff/1/sandbox/linux/services/mips_... sandbox/linux/services/mips_linux_syscalls.h:1411: On 2014/08/20 21:34:21, jln wrote: > Same remark: please fill or document hole. Done. https://codereview.chromium.org/494743003/diff/1/sandbox/linux/services/x86_3... File sandbox/linux/services/x86_32_linux_syscalls.h (right): https://codereview.chromium.org/494743003/diff/1/sandbox/linux/services/x86_3... sandbox/linux/services/x86_32_linux_syscalls.h:1396: On 2014/08/20 21:34:21, jln wrote: > Same remark: please fill or document hole. Done. https://codereview.chromium.org/494743003/diff/1/sandbox/linux/services/x86_6... File sandbox/linux/services/x86_64_linux_syscalls.h (right): https://codereview.chromium.org/494743003/diff/1/sandbox/linux/services/x86_6... sandbox/linux/services/x86_64_linux_syscalls.h:1264: On 2014/08/20 21:34:21, jln wrote: > Same remark: please fill or document hole. Done.
I think we should continue passing PROCESS_XX_THREADED for now and strictly assert it. WDYT? https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/seccomp... File sandbox/linux/seccomp-bpf/sandbox_bpf.cc (right): https://chromiumcodereview.appspot.com/494743003/diff/1/sandbox/linux/seccomp... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:481: thread_state = PROCESS_MULTI_THREADED; On 2014/08/21 16:50:18, rsesek wrote: > I actually think SandboxThreadState should just go away, since we have an easy > way to test whether or not a system supports tsync. Back when we thought the > interface was going to be prctl(), this wasn't the case. At a high level, paranoia dictates to carefully assert that the situation is exactly as the caller think it is. This is especially true while TSYNC is not a strict Chrome run-time requirement. I can imagine this changing in a year or so. In more details: - I think the caller should always know whether it is monothreaded or multithreaded. And we should enforce this with a sanity check. a. Callers that use PROCESS_SINGLE_THREADED are guaranteed to lead to a working sandbox on every kernel, not just the ones with TSYNC. This is especially important / useful on Linux (presumably we'll have TSYNC everywhere on Chrome OS). b. More problems can exist with multi-threaded initialization. Other threads could race to open new resources that are not accounted for (moving "open resources" sanity checks to after the sandbox is engaged is tricky in some cases, even though we already to it for most file descriptors). - Thread detection is not very reliable. It would be easy to make an error where thread detection is not working at all, except on bots / machines with TSYNC support. For instance, for security reasons, we only have /proc available after the setuid sandbox is engaged in DEBUG mode. In this case, we would be in a situation where this code would detect multiple threads but not fail because of TSYNC on our bots, but wouldn't detect existing threads in the field where TSYNC is not available. - We are dicussing the possibiliy of convoluted CFI + seccomp sandboxes where one thread would be "trusted" and not under seccomp-bpf. To support this, we may want to add a new PROCESS_UNTRUSTED_THREAD or something along these lines to engage seccomp-bpf on one thread in a multi-threaded process. https://chromiumcodereview.appspot.com/494743003/diff/40001/sandbox/linux/sec... File sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc (right): https://chromiumcodereview.appspot.com/494743003/diff/40001/sandbox/linux/sec... sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:158: BPF_ASSERT(syscall(__NR_nanosleep, &ts, NULL) == -1); In theory despite the 0 time you should still handle EINTR. https://chromiumcodereview.appspot.com/494743003/diff/40001/sandbox/linux/sec... sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2186: BlacklistNanosleepPolicy::AssertNanosleepFails(); What I was suggesting was to return a bool and run that function before engaging the sandbox as well. That way you can test the function itself: it returns true before the sandbox is engaged, it returns false after the sandbox is engaged. I've seen tests like this become no-op before because the function was always failing due to invalid parameters. It's a bit of a detail, so up to you.
You've convinced me; I now assert the right thread_state and added tests for that. https://chromiumcodereview.appspot.com/494743003/diff/40001/sandbox/linux/sec... File sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc (right): https://chromiumcodereview.appspot.com/494743003/diff/40001/sandbox/linux/sec... sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:158: BPF_ASSERT(syscall(__NR_nanosleep, &ts, NULL) == -1); On 2014/08/21 18:47:09, jln wrote: > In theory despite the 0 time you should still handle EINTR. Done. https://chromiumcodereview.appspot.com/494743003/diff/40001/sandbox/linux/sec... sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2186: BlacklistNanosleepPolicy::AssertNanosleepFails(); On 2014/08/21 18:47:09, jln wrote: > What I was suggesting was to return a bool and run that function before engaging > the sandbox as well. > > That way you can test the function itself: it returns true before the sandbox is > engaged, it returns false after the sandbox is engaged. > > I've seen tests like this become no-op before because the function was always > failing due to invalid parameters. > > It's a bit of a detail, so up to you. I didn't turn it into a function, since I like being able to reuse AssertNanosleepFails(), but I did add this piece of the test.
Sorry if I was not clear: I still thought that using TSYNC whenever it is available was a good idea. Could you restore that behavior? https://chromiumcodereview.appspot.com/494743003/diff/80001/sandbox/linux/sec... File sandbox/linux/seccomp-bpf/sandbox_bpf.cc (right): https://chromiumcodereview.appspot.com/494743003/diff/80001/sandbox/linux/sec... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:443: int rv = syscall(__NR_seccomp); Let's pass the NULL parameters explicitly. I'm not sure every libc guarantees 0 for missing parameters. https://chromiumcodereview.appspot.com/494743003/diff/80001/sandbox/linux/sec... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:491: SANDBOX_DIE("Cannot start sandbox; " This is fine, but I feel much less strongly about this. Being able to say "This process *could* be multithreaded" could be useful. https://chromiumcodereview.appspot.com/494743003/diff/80001/sandbox/linux/sec... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:576: if (thread_state == PROCESS_MULTI_THREADED) { It's a very nice safeguard to always use TSYNC and the new interface when it's available. This condition should depend on the result of SupportsSeccompThreadFilterSynchronization(), not on PROCESS_MULTI_THREADED. https://chromiumcodereview.appspot.com/494743003/diff/80001/sandbox/linux/sec... File sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc (right): https://chromiumcodereview.appspot.com/494743003/diff/80001/sandbox/linux/sec... sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2181: // Test that nanoseelp success. s/success succeeds. https://chromiumcodereview.appspot.com/494743003/diff/80001/sandbox/linux/sec... sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2183: BPF_ASSERT(HANDLE_EINTR(syscall(__NR_nanosleep, &ts, NULL)) == 0); BPF_ASSERT_EQ
Also rebased in the last patch set. https://codereview.chromium.org/494743003/diff/80001/sandbox/linux/seccomp-bp... File sandbox/linux/seccomp-bpf/sandbox_bpf.cc (right): https://codereview.chromium.org/494743003/diff/80001/sandbox/linux/seccomp-bp... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:443: int rv = syscall(__NR_seccomp); On 2014/08/21 21:04:42, jln wrote: > Let's pass the NULL parameters explicitly. I'm not sure every libc guarantees 0 > for missing parameters. Done. https://codereview.chromium.org/494743003/diff/80001/sandbox/linux/seccomp-bp... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:491: SANDBOX_DIE("Cannot start sandbox; " On 2014/08/21 21:04:41, jln wrote: > This is fine, but I feel much less strongly about this. Being able to say "This > process *could* be multithreaded" could be useful. Done. https://codereview.chromium.org/494743003/diff/80001/sandbox/linux/seccomp-bp... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:576: if (thread_state == PROCESS_MULTI_THREADED) { On 2014/08/21 21:04:42, jln wrote: > It's a very nice safeguard to always use TSYNC and the new interface when it's > available. > > This condition should depend on the result of > SupportsSeccompThreadFilterSynchronization(), not on PROCESS_MULTI_THREADED. Done. https://codereview.chromium.org/494743003/diff/80001/sandbox/linux/seccomp-bp... File sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc (right): https://codereview.chromium.org/494743003/diff/80001/sandbox/linux/seccomp-bp... sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2181: // Test that nanoseelp success. On 2014/08/21 21:04:42, jln wrote: > s/success succeeds. Done. https://codereview.chromium.org/494743003/diff/80001/sandbox/linux/seccomp-bp... sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2183: BPF_ASSERT(HANDLE_EINTR(syscall(__NR_nanosleep, &ts, NULL)) == 0); On 2014/08/21 21:04:42, jln wrote: > BPF_ASSERT_EQ Done.
https://codereview.chromium.org/494743003/diff/80001/sandbox/linux/seccomp-bp... File sandbox/linux/seccomp-bpf/sandbox_bpf.cc (right): https://codereview.chromium.org/494743003/diff/80001/sandbox/linux/seccomp-bp... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:443: int rv = syscall(__NR_seccomp); On 2014/08/25 17:17:26, rsesek wrote: > On 2014/08/21 21:04:42, jln wrote: > > Let's pass the NULL parameters explicitly. I'm not sure every libc guarantees > 0 > > for missing parameters. > > Done. Actually not done, I couldn't get this to work on Android.
Another comment. Sorry for being so nit-picky here, but I'm paranoid about this path. https://chromiumcodereview.appspot.com/494743003/diff/160001/sandbox/linux/se... File sandbox/linux/seccomp-bpf/sandbox_bpf.cc (right): https://chromiumcodereview.appspot.com/494743003/diff/160001/sandbox/linux/se... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:573: if (SupportsSeccompThreadFilterSynchronization() == STATUS_AVAILABLE) { I worry that if SupportsSeccompThreadFilterSynchronization() doesn't return STATUS_AVAILABLE for whatever reason here, but did before in StartSandbox(), we will enable a single thread sandbox in a multi-threaded context. We should only call SupportsSeccompThreadFilterSynchronization() once in the StartSandbox() path. How about we replace the |thread_state| argument to InstallFilter (now unused) by "bool must_sync_threads" And in StartSandbox, we would have something like must_sync_thread = sync_thread_is_supported || thread_state == PROCESS_MULTI_THREADED);
https://chromiumcodereview.appspot.com/494743003/diff/160001/sandbox/linux/se... File sandbox/linux/seccomp-bpf/sandbox_bpf.cc (right): https://chromiumcodereview.appspot.com/494743003/diff/160001/sandbox/linux/se... sandbox/linux/seccomp-bpf/sandbox_bpf.cc:573: if (SupportsSeccompThreadFilterSynchronization() == STATUS_AVAILABLE) { On 2014/08/25 18:13:49, jln wrote: > I worry that if SupportsSeccompThreadFilterSynchronization() doesn't return > STATUS_AVAILABLE for whatever reason here, but did before in StartSandbox(), we > will enable a single thread sandbox in a multi-threaded context. > > We should only call SupportsSeccompThreadFilterSynchronization() once in the > StartSandbox() path. > > How about we replace the |thread_state| argument to InstallFilter (now unused) > by "bool must_sync_threads" > > And in StartSandbox, we would have something like must_sync_thread = > sync_thread_is_supported || thread_state == PROCESS_MULTI_THREADED); Done.
lgtm
The CQ bit was checked by rsesek@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rsesek@chromium.org/494743003/180001
The CQ bit was checked by rsesek@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rsesek@chromium.org/494743003/200001
Message was sent while issue was closed.
Committed patchset #7 (200001) as e320c4dfad9a446c3fce8fed2a91d1601d7c9b69
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/07884070ecfe2e6ad00a5493dfdc4df891bbd142 Cr-Commit-Position: refs/heads/master@{#291739} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
