nxia@ has clobbered the builder to see if it fixes things.

Assigned to deputy to track. This hasn't passed on > 200 builds...

Thanks, build packages is passing now and build failure is back to bluez UnitTest fails.

The build started at 22 Aug 2017 11:18:22 -0700 (PDT) (https://build.chromium.org/p/chromiumos/builders/amd64-generic-asan/builds/20972) still failed with

telemetry-0.0.1-r6: >>> Unpacking ./dep-telemetry_dep.tar.bz2 to /build/amd64-generic/tmp/portage/chromeos-base/telemetry-0.0.1-r6/work
telemetry-0.0.1-r6:  * ACCESS DENIED:  open_wr:      /build/amd64-generic/tmp/portage/chromeos-base/telemetry-0.0.1-r6/work/dep-telemetry_dep.tar.bz2

I believe the issue is telemetry package is not able to handle reinstallation.

telemetry-0.0.1-r6: >>> Unpacking ./dep-telemetry_dep.tar.bz2 to /build/amd64-generic/tmp/portage/chromeos-base/telemetry-0.0.1-r6/work
telemetry-0.0.1-r6:  * ACCESS DENIED:  open_wr:      /build/amd64-generic/tmp/portage/chromeos-base/telemetry-0.0.1-r6/work/dep-telemetry_dep.tar.bz2
telemetry-0.0.1-r6: tar: ./dep-telemetry_dep.tar.bz2: Cannot open: Permission denied
telemetry-0.0.1-r6: tar: Exiting with failure status due to previous errors
telemetry-0.0.1-r6:  * ERROR: chromeos-base/telemetry-0.0.1-r6::chromiumos failed (unpack phase):

So, this is not a test/infra issue; this is a bad ebuild issue.

There were some semi-recent changes to enforce better isolation of ebuilds. We aren't allowed to make write-access outside of select directories. In this case, it looks like we're trying to open this file as writeable:

/build/amd64-generic/tmp/portage/chromeos-base/telemetry-0.0.1-r6/work/dep-telemetry_dep.tar.bz2

That's symlinked into our work directory, and we shouldn't be writing to that file (or directory) -- only unpacking it (with 'pbunzip2 ... | tar -xof -').

I'm a little confused then, since that *shouldn't* be trying to write there.

Simran was the original author, but this was something like 6 years ago.

You probably need to change these two lines for your new security restrictions.

https://cs.corp.google.com/chromeos_public/src/third_party/chromiumos-overlay/chromeos-base/telemetry/telemetry-0.0.1.ebuild?q=dep-telemetry_dep&dr=C&l=22

Are those lines a problem? This ebuild should have *read* access to that tarball (shouldn't it?), but it just shouldn't be *writing* to it. And from my reading, that code should not *write* to the file that the sandbox error is pointing at.

BTW, I can't repro this on my local workstation.

It's also weird that libsandbox is blaming the 'tar' command for this, when it should only be 'pbunzip2' that's touching that file.

Is there any way to get access so I can check on the builder itself? Could it somehow be doing something different with the unpack step there, that I don't understand?

I just ran into this on a synced tree while building chromeos with USE=chrome_internal.

@Comment #13: Sounds like despite it not writing "/build/samus/tmp/portage/chromeos-base/telemetry-0.0.1-r6/work/dep-telemetry_dep.tar.bz2", tar still opens it as R+W. Could it be that the changes you suggested (Comment #9) are now flagging this as an error (while earlier it just worked because nothing was denying the R+W access). Is there a way to switch off the change referenced in Comment #9 in a local build to see if it helps?

> Is there a way to switch off the change referenced in Comment #9 in a local build to see if it helps?

You'd have to ask Mike, since I never figured out exactly where the switch for that feature is. But the library code for it is in the portage-stable/sys-apps/sandbox ebuild AFAICT.

But...if you disable that feature, then of course the problem will go away. (The "problem" as worded is that the sandbox is noticing that we're opening with R/W access.) So I'm not sure that's very helpful, unless we're considering disabling the sandbox for this ebuild as a hack...

If you're reproducing the issue, getting an emerge log with --debug might help. (e.g., `emerge-${BOARD} --debug telemetry`)

Also, you can try emerge'ing while setting the variable 'FEATURES=noclean' (e.g., `FEATURES=noclean emerge-${BOARD} --debug telemetry`), so you can examine the directory structure afterward.

> tar still opens it as R+W

It seems like this would imply tar somehow got confused into unpacking the tarball...on top of itself? I'm not sure how that would have happened.

I've attached a file here with the output of the emerge.

Deleted: dump.txt 79.4 KB

dump.txt 79.4 KB View Download

@Comment  16:

> It seems like this would imply tar somehow got confused into unpacking the tarball...on top of itself? I'm not sure how that would have happened.

I'm afraid I didn't completely understand the above. It is symlinked into the work directory, and it then unpacks it in the same working directory that the tarball is symlinked to. That seemed to be ok (but I may be missing something).

Re #18:
The log claims that it's 'tar xof -' that's opening ./dep-telemetry_dep.tar.bz2 as R/W. Note that it just reads the tarball from stdout (passed there by pbunzip2) and extracts the result to the current dir.
That would be possible if dep-telemetry_dep.tar.bz2 itself contained dep-telemetry_dep.tar.bz2 inside.

So, there was a similar bug which was seen, but :
https://chromium-review.googlesource.com/c/chromiumos/third_party/autotest/+/498090 is supposed to have fixed that (and that was submitted on May 10). Is there a mismatch where the dependency hasn't been upreved since then, and so it didn't get built correctly?

In fact, crbug:718626 was for the exact same issue. I wonder why this cropping up again (after more than 3 months).

This would be a simple test (if you're still reproducing) to see if it goes away:

diff --git a/chromeos-base/telemetry/telemetry-0.0.1.ebuild b/chromeos-base/telemetry/telemetry-0.0.1.ebuild
index e1515a2e7ccc..80d082f83499 100644
--- a/chromeos-base/telemetry/telemetry-0.0.1.ebuild
+++ b/chromeos-base/telemetry/telemetry-0.0.1.ebuild
@@ -19,7 +19,7 @@ RDEPEND="dev-python/psutil"
 S=${WORKDIR}
 
 src_unpack() {
-	ln -s "${SYSROOT}${AUTOTEST_BASE}/packages/dep-telemetry_dep.tar.bz2" .
+	cp "${SYSROOT}${AUTOTEST_BASE}/packages/dep-telemetry_dep.tar.bz2" .
 	unpack ./dep-telemetry_dep.tar.bz2
 	# Some telemetry code hardcodes in 'src'
 	mv test_src src || die


You could also examine the contents of your current ${SYSROOT}${AUTOTEST_BASE}/packages/dep-telemetry_dep.tar.bz2 to see if it does indeed contain another tarball of the same name.

I inspected the tarball and it does contain the tarball itself (just like in crbug:718626). The weird thing is I already submitted a fix for this back in may.  Most of the autotest packages don't exhibit this issue, but I think there may be an issue where some ebuilds don't get upreved, so they don't get built again. This appears to be one of them.

Can we uprev the dep-telemetry_dep ebuild? That would make this problem go away.

There could be other deps which just haven't been upreved since this fix (I randomly picked dep-touchpad-tests.tar.bz2 and it exhibits the same issue).

> Can we uprev the dep-telemetry_dep ebuild? That would make this problem go away.

Sure, that sounds reasonable to me. Except...I can't even tell what ebuild that should be right now. Maybe I'm just being stupid. But equery can't pinpoint it, maybe because the autotest ebuilds do weird things?

I think I understand what has happened. The fix to exclude earlier tarballs during tarring didn't work well for the upload path (which deps tend to hit from the looks of it).

I've pushed a fix for review (https://chromium-review.googlesource.com/c/chromiumos/third_party/autotest/+/634627) and am doing a clean build on my machine to check whether things work.

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

commit 7c580f1e5eca61c0821cb7c2465b9ea97d68d8d6
Author: Prashant Malani <pmalani@google.com>
Date: Sat Aug 26 02:54:14 2017

packager: Fix exclude string for upload path

The upload path has two issues with it:
- The exclude string doesn't apply to it, so if a new tarball needs
  to be created, the earlier tarball and checksum will be included.
  leading to files growing exponentially in size.
- The checks for recomputation of tarballs are not perfect. Ideally,
  only one instance of the tarball should exist (either the one which
  was created locally, or the one that was downloaded as a prebuilt).

So, this patch solely checks if the tarball already exists. *But*, it
compares the checksum against what is in packages.checksum. If it's
different, it performs the upload.

BUG=chromium:716151
BUG= chromium:756957 
TEST=emerge-samus autotest-deps-touchpad and ensure that the tarball
doesn't contain another tarball.

Change-Id: Idd9b3eca0f6ec2680c9d26577d08ccc38501d784
Reviewed-on: https://chromium-review.googlesource.com/634627
Commit-Ready: Prashant Malani <pmalani@google.com>
Tested-by: Prashant Malani <pmalani@google.com>
Reviewed-by: Aviv Keshet <akeshet@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/7c580f1e5eca61c0821cb7c2465b9ea97d68d8d6/utils/packager.py

Whoo! This builder made it past BuildPackages:

https://build.chromium.org/p/chromiumos/builders/amd64-generic-asan/builds/21066