New issue
Advanced search Search tips

Issue 889724 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Upstart variable import filtering doesn't work correctly

Project Member Reported by mnissler@chromium.org, Sep 27

Issue description

Turns out that the upstart variable filtering implemented per  issue 818032  is buggy currently: It imports *all* passed variables if there is at least one import declaration, but no variables otherwise.
 
Project Member

Comment 1 by sheriffbot@chromium.org, Sep 27

Labels: Target-70 M-70
Project Member

Comment 2 by sheriffbot@chromium.org, Sep 27

Labels: -Pri-2 Pri-1
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 28

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

commit ed115837d671199cb26e8491b4b30ed2bb94d30b
Author: Mattias Nissler <mnissler@chromium.org>
Date: Fri Sep 28 17:10:51 2018

arc-setup: Add more variable imports to arc-boot-continue.conf

These are used, but were missing import declarations. Things didn't
break because of a bug on the upstart side. Add proper import
declarations in preparation to fixing the bug.

BUG= chromium:889724 
TEST=Passes cheets_StartAndroid.smoke

Change-Id: Ifd0e91d6c866c0f24e382e0afbdcfa22c5d8267d
Reviewed-on: https://chromium-review.googlesource.com/1247745
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/ed115837d671199cb26e8491b4b30ed2bb94d30b/arc/setup/etc/arc-boot-continue.conf

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 2

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

commit c201e5d1d388e460a1a35eec87edd83d4439d09c
Author: Mattias Nissler <mnissler@chromium.org>
Date: Tue Oct 02 16:17:39 2018

sys-apps/upstart: Fix import processing.

Fix the code that checks whether variable matches an import
declaration. The current code would incorrectly import *all* variables
for jobs that have at least one import declaration.

BUG= chromium:889724 
TEST=Image boots and passes tests.
CQ-DEPEND=CL:1247745

Change-Id: I9f2883edc55aa8b6c4ff68a4fb82b0b1a8aca080
Reviewed-on: https://chromium-review.googlesource.com/1247642
Commit-Ready: Mattias Nissler <mnissler@chromium.org>
Tested-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[rename] https://crrev.com/c201e5d1d388e460a1a35eec87edd83d4439d09c/sys-apps/upstart/upstart-1.2-r30.ebuild
[modify] https://crrev.com/c201e5d1d388e460a1a35eec87edd83d4439d09c/sys-apps/upstart/files/upstart-1.2-import-env.patch

Components: OS>Systems
Should we mark as fixed and ask for merge to M70?
Status: Fixed (was: Started)
Seems to have stuck, so marking as fixed. Not sure it's worth merging - the benefit is limited and there is a risk of breakage that we're not aware of yet.
Project Member

Comment 7 by sheriffbot@chromium.org, Oct 5

Labels: Restrict-View-SecurityNotify
Labels: Release-0-M70
Project Member

Comment 9 by sheriffbot@chromium.org, Jan 11

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment