New issue
Advanced search Search tips

Issue 657508 link

Starred by 0 users

Issue metadata

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



Sign in to add a comment

minijail seccomp parser mishandles 32-bit unsigned numbers on 32-bit systems

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

Issue description

i have some ioctls that don't fit into a 32-bit signed value.  the seccomp filter looks like:
ioctl: arg1 == 0x80045707 || arg1 == 0xc0045706 || arg1 == 0x80045702 || arg1 == 0x80045705 || arg1 == 0x5401

running my program through it fails with SIGSYS when the ioctl is called.  playing around with things, if i use the signed values, it works:
ioctl: arg1 == -2147199225 || arg1 == -1073457402 || arg1 == -2147199230 || arg1 == -2147199227 || arg1 == 0x5401

looks like the parser can only handle 32-bit signed inputs.  prob this bit of code in util.c:
long int parse_single_constant(char *constant_str, char **endptr)
{   
    ...
    return strtol(constant_str, endptr, 0);
}

but i'm just guessing ... haven't dug into the minijail runtime.
 
Maybe we need to use strtoll so that things work correctly on 32-bit systems.
Actually, I'm pretty sure we need to use strtoll so that things work correctly on 32-bit systems. sizeof(long int) on 32-bit systems is 4, so we cannot parse unsigned quantities larger than 2^31-1.
Status: Started (was: Available)
https://android-review.googlesource.com/#/c/291626/ is up and should land soon.
Status: Fixed (was: Started)

Comment 5 by vapier@chromium.org, Oct 20 2016

going to do a CrOS uprev for me ? :)
Hah. I can uprev.
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 21 2016

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

commit 6ea4ed572d91e59d2c140d125bb342fabea1cd14
Author: Jorge Lucangeli Obes <jorgelo@chromium.org>
Date: Thu Oct 20 18:33:27 2016

Uprev Minijail.

Changes:
0745937 Fix parsing of unsigned constants.
5389540 Fix invalid constant error reporting.

BUG= chromium:657508 
TEST=security_Minijail0, security_Minijail_seccomp

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

[rename] https://crrev.com/6ea4ed572d91e59d2c140d125bb342fabea1cd14/chromeos-base/chromeos-minijail/chromeos-minijail-0.0.1-r1469.ebuild

Comment 8 by dchan@google.com, Jan 21 2017

Labels: VerifyIn-57

Comment 9 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 10 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 11 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 13 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)
Components: OS>Systems>Minijail
Status: Fixed (was: Archived)

Sign in to add a comment