New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 657352 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

All canaries failed when building minijail

Project Member Reported by cychiang@chromium.org, Oct 19 2016

Issue description

https://chromium-review.googlesource.com/#/c/399891/ can not merge because minijail-init is not in the whitelist of security_SandboxedServices
 
Summary: All canaries failed when building minijail (was: security_SandboxedServices failed because of minijail-init)
We need to chump in https://chromium-review.googlesource.com/#/c/399891/ so minijail pointing to the correct tree.

E.g.

https://chromegw.corp.google.com/i/chromeos/builders/auron_yuna-release/builds/497/steps/BuildPackages%20%5Bafdo_use%5D/logs/stdio


chromeos-minijail-0.0.1-r1467: >>> Emerging (1 of 1) chromeos-base/chromeos-minijail-0.0.1-r1467::chromiumos for /build/auron_yuna/
chromeos-minijail-0.0.1-r1467:  * Running stacked hooks for pre_pkg_setup
chromeos-minijail-0.0.1-r1467:  *    sysroot_build_bin_dir ...
chromeos-minijail-0.0.1-r1467:  [ ok ]
chromeos-minijail-0.0.1-r1467:  * Running stacked hooks for pre_src_unpack
chromeos-minijail-0.0.1-r1467:  *    python_multilib_setup ...
chromeos-minijail-0.0.1-r1467:  [ ok ]
chromeos-minijail-0.0.1-r1467: >>> Unpacking source...
chromeos-minijail-0.0.1-r1467: Cloning into '/build/auron_yuna/tmp/portage/chromeos-base/chromeos-minijail-0.0.1-r1467/work/chromeos-minijail-0.0.1'...
chromeos-minijail-0.0.1-r1467: done.
chromeos-minijail-0.0.1-r1467: fatal: reference is not a tree: 472581ace012ae755f3136870bba8bdd5cdb8331
chromeos-minijail-0.0.1-r1467:  * Cannot run git checkout 472581ace012ae755f3136870bba8bdd5cdb8331 in /build/auron_yuna/tmp/portage/chromeos-base/chromeos-minijail-0.0.1-r1467/work/chromeos-minijail-0.0.1.
chromeos-minijail-0.0.1-r1467:  * Is /mnt/host/source/src/aosp/external/minijail up to date? Try running repo sync.
chromeos-minijail-0.0.1-r1467:  * Falling back to git.eclass...
chromeos-minijail-0.0.1-r1467: From https://android.googlesource.com/platform/external/minijail


Along with the change, we need to add minijail-init into security_SandboxedServices whitelist.
Cc: lhchavez@google.com
I am going to chump these two CL in.

https://chromium-review.googlesource.com/#/c/399891/
https://chromium-review.googlesource.com/#/c/400203/

If anyone thinks this might be wrong please revert.
Thanks!
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/3209683c9920ecc67e9a3b7de2bb45015f1463ab

commit 3209683c9920ecc67e9a3b7de2bb45015f1463ab
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Wed Oct 19 11:47:26 2016

security_SandboxedServices: Add minijail-init to baseline

Add minijail-init to baseline whitelist for minijail change.
https://chromium-review.googlesource.com/#/c/399891/

BUG= chromium:657352 
TEST=run the test against DUT with that change.

Change-Id: I41a12bca87422f939406b27bfa47edf1a5b8b5ab
Reviewed-on: https://chromium-review.googlesource.com/400203
Reviewed-by: Cheng-Yi Chiang <cychiang@chromium.org>
Commit-Queue: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>

[modify] https://crrev.com/3209683c9920ecc67e9a3b7de2bb45015f1463ab/client/site_tests/security_SandboxedServices/baseline

Huh, why did the original manifest change land without the revision change?
I think the CQ-DEPEND was wrong. The git hash update CL was made to depend on the manifest change, but there were no CQ-DEPENDs the other way around, so the manifest change was allowed to land without the Minijail uprev.
Status: Fixed (was: Started)
Also, it looks like Jimmy's change fixed this. I successfully built locally, the build boots and passes Minijail tests. I think we can close this bug.
Right..FYI, if you have CLs A,B,C that need to be merged in the same build, you can let
A CQ-DEPEND B
B CQ-DEPEND C
C CQ-DEPEND A

Thanks!
Yeah, we should have made them all depend on each other. For some reason I initially thought that the manifest changes could go in without an update for the hashes.
Sorry, probably stupid question, but how did the first change, or set of changes, get past the paladins?
Not a stupid question, I asked the same thing in https://chromium-review.googlesource.com/#/c/400478/. It looks like the PreCQ has a slightly different failure mode than the CQ.
Sorry, how does this apply here?  Did the PreCQ fail, and the CQ not fail?
The other way around. PreCQ succeeded on those CLs, CQ failed. Moreover, due to an incorrect set of CQ-DEPEND lines, the failing CQ only rejected one of the four related CLs.
But doesn't a failing CQ reject ALL of the incoming CLs?
Ah no I was wrong. The aforementioned CQ-DEPEND mistake caused the CLs to go into *different* CQ runs. The CL that didn't land was not marked Presubmit Ready until after the CQ run that took the other three CLs.

I still don't know the answer to your original question: why the CQ run here: https://uberchromegw.corp.google.com/i/chromeos/builders/master-paladin/builds/12656
passed the paladins only to apparently fail afterwards.
Ah that makes sense, thanks a lot.  I am working on the second question, will post here when I have an answer.  My current theory is that the test that failed in the canaries isn't run on the CQ, so this may be WAI.  (CQ needs to be faster.)
Mmm and no, right, it wasn't a test failure :/
Yeah, as I mentioned on email, the problem is that a group of three CLs that ended up causing the build failure in the original post of this bug went through a CQ run correctly.
Labels: -Restrict-View-Google
Project Member

Comment 20 by bugdroid1@chromium.org, Oct 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/733bf7bc1c32e34a989991874a9a5b8029f6750f

commit 733bf7bc1c32e34a989991874a9a5b8029f6750f
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Oct 19 16:46:40 2016

security_SandboxedServices: handle minijail-init

The new minijail code will rename itself as "minijail-init" so it's clear
what its job is.  Update the parsing logic to handle that in addition to
the old minijail0 invocation.

BUG= chromium:657352 
TEST=precq passes

Change-Id: I1eeb439246389d724381375459d92562e4ec1373
Reviewed-on: https://chromium-review.googlesource.com/400540
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[modify] https://crrev.com/733bf7bc1c32e34a989991874a9a5b8029f6750f/client/site_tests/security_SandboxedServices/exclude
[modify] https://crrev.com/733bf7bc1c32e34a989991874a9a5b8029f6750f/client/site_tests/security_SandboxedServices/baseline
[modify] https://crrev.com/733bf7bc1c32e34a989991874a9a5b8029f6750f/client/site_tests/security_SandboxedServices/security_SandboxedServices.py

Project Member

Comment 21 by bugdroid1@chromium.org, Oct 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/c7c337804fdfb0f9c5ad251978f74131dc22769b

commit c7c337804fdfb0f9c5ad251978f74131dc22769b
Author: Mike Frysinger <vapier@chromium.org>
Date: Fri Oct 13 21:45:07 2017

security_SandboxedServices: document the baseline file format

BUG= chromium:657352 
TEST=precq passes

Change-Id: I92826dbb24d4daaf37f91588c82648d9e1e29476
Reviewed-on: https://chromium-review.googlesource.com/401888
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[modify] https://crrev.com/c7c337804fdfb0f9c5ad251978f74131dc22769b/client/site_tests/security_SandboxedServices/baseline.lakitu
[modify] https://crrev.com/c7c337804fdfb0f9c5ad251978f74131dc22769b/client/site_tests/security_SandboxedServices/baseline
[modify] https://crrev.com/c7c337804fdfb0f9c5ad251978f74131dc22769b/client/site_tests/security_SandboxedServices/baseline.x86-zgb
[modify] https://crrev.com/c7c337804fdfb0f9c5ad251978f74131dc22769b/client/site_tests/security_SandboxedServices/baseline.lakitu-gpu
[modify] https://crrev.com/c7c337804fdfb0f9c5ad251978f74131dc22769b/client/site_tests/security_SandboxedServices/baseline.whirlwind
[modify] https://crrev.com/c7c337804fdfb0f9c5ad251978f74131dc22769b/client/site_tests/security_SandboxedServices/baseline.veyron_rialto
[modify] https://crrev.com/c7c337804fdfb0f9c5ad251978f74131dc22769b/client/site_tests/security_SandboxedServices/baseline.moblab

Sign in to add a comment