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

Issue 870321 link

Starred by 1 user

Issue metadata

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

Blocked on:
issue 871668



Sign in to add a comment

CRAS: Add seccomp policy files to adhd/cras

Project Member Reported by hychao@chromium.org, Aug 2

Issue description

I've uploaded CL to add seccomp policy files in https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/1136345/2
but things are not working smooth for the ioctl syscall :(

Basically when I specify all the ioctl arguments in arg whiltelist, minijail0/cras crashes.
And I just figured that out there're two issues in it:

(1) minijail has line length limit 1024, per:
https://chromium.googlesource.com/chromiumos/platform/minijail/+/master/syscall_filter.c#14
and we just have so many arguments to whitelist, as in asound.h
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.4/include/uapi/sound/asound.h#260

(2) After I doubled the limit to 2048, I still see CRAS got blocked at ioctl syscall.
Turns out it's a problem in alsa-lib, where SNDRV_CTL_IOCTL_TLV_* first set to an local int, and then pass to ioctl() where "unsigned long" arg1 is accepted.

http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/control/control_hw.c#l238

so we should fix (1) (2) above, and then we're close to land the seccomp policy files.
 
CRAS is now calling 'metrics_client' to send data to UMA, and that requires some more system calls to be added to seccomp policy file.

Change CRAS to use libmetrics makes it simpler.

https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1163643 metrics: Add C wrapper for SendCrosEventToUMA
https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/1163644 adhd: depends on chromeos-base/metrics
https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/1163645 CRAS: metrics - Use libmetrics instead of calling metrics_client

Blockedon: 871668
Cc: dklee@google.com
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/c63919d7c78f57739bd9bc509c75b5de107d8aad

commit c63919d7c78f57739bd9bc509c75b5de107d8aad
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Thu Aug 30 16:52:52 2018

alsa-lib: Apply upstream patches

This change copies alsa-lib-1.1.6 from portage-stable and
applies below patch from upstream.

7c5c0500 seq: Fix signedness in MIDI encoder/decoder
a8491636 control_hw: Fix issue when applying seccomp policy

BUG= chromium:870321 ,  chromium:843791 
TEST=emerge and deploy alsa-lib, apply seccomp policy file
for adhd to verify ioctl doesn't get blocked.
Run midis unit tests with fuzzer and asan.

Change-Id: Ide0fcf97a3c4c3f20cde52d9964cb75a87d3fc13
Reviewed-on: https://chromium-review.googlesource.com/1175652
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Prashant Malani <pmalani@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[add] https://crrev.com/c63919d7c78f57739bd9bc509c75b5de107d8aad/media-libs/alsa-lib/alsa-lib-1.1.6-r2.ebuild
[add] https://crrev.com/c63919d7c78f57739bd9bc509c75b5de107d8aad/media-libs/alsa-lib/files/0001-control_hw-Fix-issue-when-applying-seccomp-policy.patch
[add] https://crrev.com/c63919d7c78f57739bd9bc509c75b5de107d8aad/media-libs/alsa-lib/files/alsa-lib-1.1.6-missing_files.patch
[add] https://crrev.com/c63919d7c78f57739bd9bc509c75b5de107d8aad/media-libs/alsa-lib/files/0002-seq-Fix-signedness-in-MIDI-encoder-decoder.patch
[add] https://crrev.com/c63919d7c78f57739bd9bc509c75b5de107d8aad/media-libs/alsa-lib/Manifest
[add] https://crrev.com/c63919d7c78f57739bd9bc509c75b5de107d8aad/media-libs/alsa-lib/metadata.xml

There are 3 other CLs landed for this and I forgot to associate them with the bug:

https://chromium-review.googlesource.com/1149767
https://chromium-review.googlesource.com/1149769
https://chromium-review.googlesource.com/1136345

I'll do some test on M70 image first and request M69 merge then.
Cc: cindyb@chromium.org
Labels: Merge-Request-69
It has been a few weeks since we added seccomp files in M70, and things are looking stable.
Request merge to M69, so we can test system AEC on CFM(board Buddy).
Labels: -Merge-Request-69
Status: Fixed (was: Started)
No longer need this in 69. Mark as fixed now.

Sign in to add a comment