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

Issue 788213 link

Starred by 3 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Feature


Sign in to add a comment

Harden mosys

Project Member Reported by jclinton@chromium.org, Nov 23 2017

Issue description

This is an umbrella bug to track bringing mosys under ongoing maintenance and to harden it against code rot and security vulnerabilities. I am going to work on this.

Background: mosys is around a decade old and originates from within Google's Platforms team. Over the years, it's fallen into disrepair suffering from a tragedy of the commons. Its primary maintainer left Google in 2017 and--since then--has bounced around a bit. It has several major issues:

* An arcane build system that mixes hand-crafted Makefiles with Kbuild imported from kernel 2.6
* Ubiquitous use of function pointers, non-typesafe casts, pointer arithmetic and inconsistent code style
* No tests (except for a small one added last month by sjg@)
* Runs as root with compiled-in code and helpers to manipulate a number of system-critical ioctl's and file descriptors making any second-order attack easier to execute
* Does not run in a minijail yet is called from dozens of locations all over the CrOS codebase
* Contains lots of dead code

In any scenario, mosys will be around for the foreseeable future though its exact remit is somewhat in flux. In short, we have to do something to clean it up and harden it against further rot.

There will be a design doc forthcoming but there are some immediate cleanups that can be said to fall under this umbrella that will also land on this bug.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/92ba3c22d724f565dff065e5ba1d776966f631f1

commit 92ba3c22d724f565dff065e5ba1d776966f631f1
Author: Jason D. Clinton <jclinton@chromium.org>
Date: Fri Nov 24 22:41:01 2017

Makefile: Begin to remove a decade of cruft

This deletes:
* The not-compiling, 'hello world'-type mosys_test.c.
* Removes all references to cmockery
* Removes all references to Subversion
* Begins to remove some of the kernel 2.6 (!!) macros
  * Removes System.map generation
  * Removes depmod generation
* Updates fmap to the modern symbol name and removes some indirection
* Updates clean to remove .o files, not kernel .ko files (!!)

TEST=FEATURES=test emerge-reef mosys
BUG=chromium:788213
BRANCH=none

Change-Id: Idcc23d11487850cbee76d6d2d6bd209614dc0d19
Reviewed-on: https://chromium-review.googlesource.com/788232
Commit-Ready: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>

[delete] https://crrev.com/2da4d03a7fcfa398a5278d2f5260ebbfbd1223a1/mosys_test.c
[modify] https://crrev.com/92ba3c22d724f565dff065e5ba1d776966f631f1/Makefile

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 25 2017

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

commit 34dacf415a47b8af8153998a8c16ccacc50a565f
Author: Jason D. Clinton <jclinton@chromium.org>
Date: Sat Nov 25 04:31:21 2017

sys-apps/mosys: Remove environment hacks

These were here due to a bug in mosys Makefile which is now fixed in
dependent CL.

BUG=chromium:788213
TEST=FEATURES=test USE=static emerge-reef mosys
CQ-DEPEND=CL:788233

Change-Id: Ia6a5163abcdef32b3921f961e04878b60bbaa39c
Reviewed-on: https://chromium-review.googlesource.com/788178
Commit-Ready: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>

[modify] https://crrev.com/34dacf415a47b8af8153998a8c16ccacc50a565f/sys-apps/mosys/mosys-9999.ebuild

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 25 2017

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

commit 97107f4abc75604c12a8d487c6e9390d6438f6da
Author: Jason D. Clinton <jclinton@chromium.org>
Date: Sat Nov 25 04:31:21 2017

sys-apps/mosys: Only install mosys_s if requested

mosys Makefile always compiles the static version of mosys as mosys_s.
Because of the way this ebuild was writen, it assumed that this target
wasn't built until LDFLAGS was modified to have -static; this is false.
Instead, the nonfatal dobin was causing the static mosys_s to be
installed alongside mosys on every board costing us 1.3MB of space on
the rootfs.

If folks want to use the static version, mosys_s, they should copy that
after doing USE=static.

BUG=chromium:788213
TEST=FEATURES=test USE=static emerge-reef mosys

Change-Id: Ie56b3f3273e2576c001f76c6cd4c1a0766540a8c
Reviewed-on: https://chromium-review.googlesource.com/788179
Commit-Ready: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>

[modify] https://crrev.com/97107f4abc75604c12a8d487c6e9390d6438f6da/sys-apps/mosys/mosys-9999.ebuild

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/def617c5e8106fabea474bb8e17d94b167c4f3c1

commit def617c5e8106fabea474bb8e17d94b167c4f3c1
Author: Jason D. Clinton <jclinton@chromium.org>
Date: Sat Nov 25 04:31:21 2017

Makefile: Handle empty string environment variables

I spent hours looking at this to close an ancient bug
https://github.com/dhendrix/mosys/issues/3 opened in 2012. The reason
that this has fought being resolved for so long is that we are mixing
Makefile and Kbuild systems and somewhere something is setting
variables like CC to empty string. This caused Makefiles ?= syntax to
skip sane fallbacks logic. This updates this to test for empty string
and performs assignment if the emtpy string is there.

BUG=chromium:788213
TEST=FEATURES=test emerge-reef mosys
FEATURES=test USE=static emerge-reef mosys
BRANCH=none

Change-Id: Ic6559c25681a5f5d37cfb79684f93573c68738a1
Reviewed-on: https://chromium-review.googlesource.com/788233
Commit-Ready: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>

[modify] https://crrev.com/def617c5e8106fabea474bb8e17d94b167c4f3c1/Makefile

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/ad9fe6290f20dc3defe52e4c6c039cf1b5117337

commit ad9fe6290f20dc3defe52e4c6c039cf1b5117337
Author: Jason D. Clinton <jclinton@chromium.org>
Date: Sat Nov 25 04:31:22 2017

Makefile: Add a static library that can be imported by another FFI

Adding this library will allow us to iteratively call mosys C functions
from another language built in the same directory without rewriting all
of mosys.

BUG=chromium:788213
TEST=make defconfig; make; confirm mosys.a generated
BRANCH=none

Change-Id: I4789e8b1624c84e38d1b6a73ae63596236365ddb
Reviewed-on: https://chromium-review.googlesource.com/788234
Commit-Ready: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>

[modify] https://crrev.com/ad9fe6290f20dc3defe52e4c6c039cf1b5117337/Makefile

Cc: adurbin@chromium.org
This is great work, thanks!
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/248eec5d367fdb8adf7e39babbbcb0b21d77ac7d

commit 248eec5d367fdb8adf7e39babbbcb0b21d77ac7d
Author: Jason D. Clinton <jclinton@chromium.org>
Date: Tue Nov 28 21:34:30 2017

Makefile: Fix-up new static library to allow linking to work

I made two mistakes in the previous CL:
* All code needs to be compiled with PIC in order to enable linking
* The static library needs to be libmosys.a such that -lmosys will find
  the library.

BUG=chromium:788213
TEST=FEATURES=test USE=static emerge-reef mosys
BRANCH=none

Change-Id: Id8b1cd4fa0e7d40b819468700effac0b5f6d6f69
Reviewed-on: https://chromium-review.googlesource.com/789799
Commit-Ready: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: C Shapiro <shapiroc@google.com>

[modify] https://crrev.com/248eec5d367fdb8adf7e39babbbcb0b21d77ac7d/Makefile
[modify] https://crrev.com/248eec5d367fdb8adf7e39babbbcb0b21d77ac7d/.gitignore

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 2 2017

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

commit 0d8398cdb53af7ef59d6a22e35dfb2db6071fa9b
Author: Jason D. Clinton <jclinton@chromium.org>
Date: Sat Dec 02 06:45:32 2017

sys-apps/mosys: Use platform2 qemu to run arm tests

platform2 scripts have automation to use qemu to run simple binary
tests. This uses that automation by bringing platform2 into the ebuild
(as other ebuilds do) and invoking "platform_test run".

BUG=chromium:790786,chromium:788213
TEST=FEATURES=test USE=unibuild emerge-rainier mosys; FEATURES=test
emerge-reef mosys

Change-Id: Ib2adbb8cee2637868f9f527bbd1665cd20214235
Reviewed-on: https://chromium-review.googlesource.com/804594
Commit-Ready: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/0d8398cdb53af7ef59d6a22e35dfb2db6071fa9b/sys-apps/mosys/mosys-9999.ebuild
[modify] https://crrev.com/0d8398cdb53af7ef59d6a22e35dfb2db6071fa9b/sys-apps/mosys/files/chromeos-version.sh

Project Member

Comment 11 by bugdroid1@chromium.org, Dec 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/68406eb01f44baca0bc8419df834d7921466c774

commit 68406eb01f44baca0bc8419df834d7921466c774
Author: Jason D. Clinton <jclinton@chromium.org>
Date: Fri Dec 08 11:59:17 2017

rust: Create Rust scaffolding

This adds a Hello World main.rs that links against the libmosys.a output
of the legacy C build system. With:

  cargo build --release

the resulting binary has link-time optimization (LTO), stripped debug
symbols, and stripped stack unwinding (graceful shutdown).

BUG=chromium:788213
TEST=cargo build --release
BRANCH=none

Change-Id: I8ad5c49df0cfbdf4a4afe238f4164bc82f1266bd
Reviewed-on: https://chromium-review.googlesource.com/792119
Commit-Ready: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>

[add] https://crrev.com/68406eb01f44baca0bc8419df834d7921466c774/Cargo.lock
[add] https://crrev.com/68406eb01f44baca0bc8419df834d7921466c774/src/main.rs
[add] https://crrev.com/68406eb01f44baca0bc8419df834d7921466c774/build.rs
[add] https://crrev.com/68406eb01f44baca0bc8419df834d7921466c774/Cargo.toml
[modify] https://crrev.com/68406eb01f44baca0bc8419df834d7921466c774/.gitignore

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/f743f3f4cad0624f0ad9d2c16ce573fc3dd38763

commit f743f3f4cad0624f0ad9d2c16ce573fc3dd38763
Author: Jason D. Clinton <jclinton@chromium.org>
Date: Sat Dec 09 07:13:56 2017

rust: Wire up automatically generated bindings to mosys C functions

This uses Mozilla's Bindgen which we'll have to bring into our internal
Cargo repositories to use at build-time (without going out to the
network).

BUG=chromium:788213
TEST=cargo run && cargo test
BRANCH=none

Change-Id: I6d3c89469948f32ccb07183a5221290ebf22d839
Reviewed-on: https://chromium-review.googlesource.com/792120
Commit-Ready: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/f743f3f4cad0624f0ad9d2c16ce573fc3dd38763/Cargo.lock
[modify] https://crrev.com/f743f3f4cad0624f0ad9d2c16ce573fc3dd38763/Makefile
[modify] https://crrev.com/f743f3f4cad0624f0ad9d2c16ce573fc3dd38763/build.rs
[modify] https://crrev.com/f743f3f4cad0624f0ad9d2c16ce573fc3dd38763/Cargo.toml
[modify] https://crrev.com/f743f3f4cad0624f0ad9d2c16ce573fc3dd38763/src/main.rs

Project Member

Comment 13 by bugdroid1@chromium.org, Dec 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/61721923368615a55da239177899f432b97f159e

commit 61721923368615a55da239177899f432b97f159e
Author: Jason D. Clinton <jclinton@chromium.org>
Date: Sat Dec 09 07:13:56 2017

rust: Organize code layout in idiomatic way for a binaries

This just rearanges the Rust src/ directory to reflect normal layout.

BUG=chromium:788213
TEST=cargo test
BRANCH=none

Change-Id: I7f4f566c4e037b31acd5abc4cdc0480b4f4fa2d6
Reviewed-on: https://chromium-review.googlesource.com/792121
Commit-Ready: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>

[modify] https://crrev.com/61721923368615a55da239177899f432b97f159e/src/main.rs
[add] https://crrev.com/61721923368615a55da239177899f432b97f159e/src/lib.rs

Project Member

Comment 15 by bugdroid1@chromium.org, Dec 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/d1325a7a8cbd6d74633862015fb3f7dc0fe7458d

commit d1325a7a8cbd6d74633862015fb3f7dc0fe7458d
Author: Jason D. Clinton <jclinton@chromium.org>
Date: Thu Dec 14 01:26:08 2017

rust: Add idiomatic error encapsulation

This just follows the guide in the Rust Book.

BUG=chromium:788213
TEST=cargo test
BRANCH=none

Change-Id: Ic8d7736a577a4997f5c749750ca59271fe8100e2
Reviewed-on: https://chromium-review.googlesource.com/815387
Commit-Ready: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>

[modify] https://crrev.com/d1325a7a8cbd6d74633862015fb3f7dc0fe7458d/src/logging.rs
[modify] https://crrev.com/d1325a7a8cbd6d74633862015fb3f7dc0fe7458d/src/lib.rs

Blockedon: 795837
Blockedon: 795841
Blockedon: 795845
Blockedon: 795883
Blockedon: 795887
I've added blocker bugs for everything that I want to achieve with this initiative. Most updates will occur on those bugs for the foreseeable future.
Project Member

Comment 22 by bugdroid1@chromium.org, Dec 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/82628c488ad7a5e0e383683ff571095ca0b7aded

commit 82628c488ad7a5e0e383683ff571095ca0b7aded
Author: Jason D. Clinton <jclinton@chromium.org>
Date: Tue Dec 19 04:33:06 2017

rust: Wire up getopts and args tests

This imports getops and adds the existing commandline arguments from
mosys.c (which will be deleted).

Mosys command dispatch will be in the next CL.

BUG=chromium:788213
TEST=cargo test
BRANCH=none

Change-Id: I1388cc3bfab1dbf9f304c08584125b3ceff820ec
Reviewed-on: https://chromium-review.googlesource.com/815388
Commit-Ready: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/82628c488ad7a5e0e383683ff571095ca0b7aded/Cargo.lock
[modify] https://crrev.com/82628c488ad7a5e0e383683ff571095ca0b7aded/src/main.rs
[modify] https://crrev.com/82628c488ad7a5e0e383683ff571095ca0b7aded/src/lib.rs
[modify] https://crrev.com/82628c488ad7a5e0e383683ff571095ca0b7aded/Cargo.toml

Blockedon: 795122
Blockedon: 679858
Cc: nsanders@chromium.org
Project Member

Comment 26 by bugdroid1@chromium.org, Mar 27 2018

Labels: merge-merged-factory-fizz-10167.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/9c22f933d73667280e374f5426e989a6d17ae2df

commit 9c22f933d73667280e374f5426e989a6d17ae2df
Author: Jason D. Clinton <jclinton@chromium.org>
Date: Tue Mar 27 01:14:49 2018

Makefile: Fix-up new static library to allow linking to work

I made two mistakes in the previous CL:
* All code needs to be compiled with PIC in order to enable linking
* The static library needs to be libmosys.a such that -lmosys will find
  the library.

BUG=chromium:788213
TEST=FEATURES=test USE=static emerge-reef mosys
BRANCH=none

Change-Id: Id8b1cd4fa0e7d40b819468700effac0b5f6d6f69
Reviewed-on: https://chromium-review.googlesource.com/789799
Commit-Ready: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: C Shapiro <shapiroc@google.com>
(cherry picked from commit 248eec5d367fdb8adf7e39babbbcb0b21d77ac7d)
Reviewed-on: https://chromium-review.googlesource.com/981204
Reviewed-by: Jason Clinton <jclinton@chromium.org>
Commit-Queue: YH Lin <yueherngl@chromium.org>
Tested-by: YH Lin <yueherngl@chromium.org>

[modify] https://crrev.com/9c22f933d73667280e374f5426e989a6d17ae2df/Makefile
[modify] https://crrev.com/9c22f933d73667280e374f5426e989a6d17ae2df/.gitignore

Project Member

Comment 27 by bugdroid1@chromium.org, Apr 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/b631fec125f72c014a7d9778745f2396bef069f6

commit b631fec125f72c014a7d9778745f2396bef069f6
Author: Jason D. Clinton <jclinton@chromium.org>
Date: Tue Apr 17 00:57:46 2018

Delete the vpd_encode tool

It doesn't appear to ever be used; the last mention of it in crbug was
2012.

BUG=chromium:788213
TEST=none
BRANCH=none

Change-Id: If822513016ebe5bf31f1fdde96aa27a0ee8d59bb
Reviewed-on: https://chromium-review.googlesource.com/1012531
Commit-Ready: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Alec Thilenius <athilenius@google.com>

[delete] https://crrev.com/974d41165963acc2badd7e7e172c06113e6e4ead/tools/vpd_encode/vpd_encode.c
[delete] https://crrev.com/974d41165963acc2badd7e7e172c06113e6e4ead/tools/vpd_encode/symbol.c
[delete] https://crrev.com/974d41165963acc2badd7e7e172c06113e6e4ead/tools/vpd_encode/symbol.h
[delete] https://crrev.com/974d41165963acc2badd7e7e172c06113e6e4ead/tools/vpd_encode/vendor_blobs/google_blob.c
[delete] https://crrev.com/974d41165963acc2badd7e7e172c06113e6e4ead/tools/vpd_encode/vendor_blobs/vendor_blobs.h
[delete] https://crrev.com/974d41165963acc2badd7e7e172c06113e6e4ead/tools/vpd_encode/type1.c
[delete] https://crrev.com/974d41165963acc2badd7e7e172c06113e6e4ead/tools/vpd_encode/README
[delete] https://crrev.com/974d41165963acc2badd7e7e172c06113e6e4ead/tools/vpd_encode/lib_vpd_encode.c
[delete] https://crrev.com/974d41165963acc2badd7e7e172c06113e6e4ead/tools/vpd_encode/type0.c
[delete] https://crrev.com/974d41165963acc2badd7e7e172c06113e6e4ead/tools/vpd_encode/vendor_blobs/agz_blob.c
[delete] https://crrev.com/974d41165963acc2badd7e7e172c06113e6e4ead/tools/vpd_encode/lib_vpd_encode.h

Blockedon: 838349
Owner: shapiroc@chromium.org
Final blocker for this project is functional tests which were rollback and are currently pending some changes from Charles. Otherwise, this project is complete.

Sign in to add a comment