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

Issue 912146 link

Starred by 1 user
Status: Assigned
Owner:
█ ravisadineni@chromium.org
Last visit > 30 days ago
Cc:
norvez@chromium.org
█ lhchavez@chromium.org
mnissler@chromium.org
hidehiko@chromium.org
nya@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression



Sign in to add a comment

biod runs as uinput group instead of biod group due to minijail0 command line

Project Member Reported by derat@chromium.org, Dec 5 
http://cs/chromeos_public/src/platform2/biod/init/biod.conf passes "-u biod -g biod -g uinput" to minijail0 when running biod.

When I look at https://android.googlesource.com/platform/external/minijail/+/master/minijail0_cli.c, I don't see anything to suggest that passing multiple -g flags like this does what's likely intended here.

I think that this is making biod run in the uinput group rather than the biod group. This gives its log files the wrong permissions, which makes the security.StatefulFiles Tast test fail as discussed at https://crrev.com/c/1348614.

I think that the extra -g arg was added by https://chromium-review.googlesource.com/1279402 (which I reviewed -- whoops).

Ravi, can you fix this? I think that the correct way to add the biod user to the uinput group is by adding it to http://cs/chromeos_public/src/third_party/eclass-overlay/profiles/base/accounts/group/uinput.

Additionally, maybe minijail0 should crash when given multiple -g flags (if it's unsupported) to make this easier to catch.
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 7 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/56bbf9b26a07da23c91e4413a5ac9358cdb56ec0

commit 56bbf9b26a07da23c91e4413a5ac9358cdb56ec0
Author: RaviChandra Sadineni <ravisadineni@google.com>
Date: Fri Dec 07 06:06:48 2018

biod: run in biod group instead of uinput group

minijail does not parse multiple -g options. Current conf makes
biod run in uinput group but not in biod group. Remove the second
-g option.

BUG=chromium:912146
TEST=None

Change-Id: I50bb1caa5ecec20094ce5c98a1bdc21796095e1f
Reviewed-on: https://chromium-review.googlesource.com/1363340
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/56bbf9b26a07da23c91e4413a5ac9358cdb56ec0/biod/init/biod.conf

Comment 2 by norvez@chromium.org, Dec 7

Labels: Proj-Fingerprints
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 8 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/5cb5a77ae678c341c342e69842bf69e6442e0dbc

commit 5cb5a77ae678c341c342e69842bf69e6442e0dbc
Author: RaviChandra Sadineni <ravisadineni@google.com>
Date: Sat Dec 08 16:43:59 2018

Add biod to uinput group.

Biod creates a virtual input device to communicate with powerd.
update usergroup_baseline.py to reflect the new change.

BUG=chromium:912146
TEST=None

Change-Id: Ib29163a39d8f19430b79297d75228ed56d6495f2
Reviewed-on: https://chromium-review.googlesource.com/1368366
Commit-Ready: Ravi Chandra Sadineni <ravisadineni@chromium.org>
Tested-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
Reviewed-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>

[modify] https://crrev.com/5cb5a77ae678c341c342e69842bf69e6442e0dbc/cros/test/usergroup_baseline.py
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 8 

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

commit c7604365366da43c2dc27d5329bfdd4d03a2ad79
Author: RaviChandra Sadineni <ravisadineni@google.com>
Date: Sat Dec 08 16:43:59 2018

Add biod to uinput group

Biod creates virtual input device to communicate with powerd. Add biod
to the uinput group.

BUG=chromium:912146
TEST=None
CQ-DEPEND=CL:1368366

Change-Id: I49fd736195d9cc8c2e4b6d6e8e7fffdeaa683c8c
Reviewed-on: https://chromium-review.googlesource.com/1363535
Commit-Ready: Ravi Chandra Sadineni <ravisadineni@chromium.org>
Tested-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/c7604365366da43c2dc27d5329bfdd4d03a2ad79/profiles/base/accounts/group/uinput

Comment 6 by norvez@chromium.org, Jan 11 

I'm assuming this is fixed? Did that get merged back to M-72? Should it be?
►

Sign in to add a comment