New issue
Advanced search Search tips

Issue 878440 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 886953



Sign in to add a comment

chromeos-base/verity: fixup header conflicts in /usr/include/linux/

Project Member Reported by briannorris@chromium.org, Aug 28

Issue description

The verity package installs a bunch of forked copies of linux/ headers into /usr/include/verity/.

This causes conflicts and errors when upgrading our Linux headers, because now (depending on the compiler -I<include> paths) the verity headers might be missing stuff that dependent headers required.

See, for instance, with this upgrade attempt:

https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/1191991/2

https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/buildDetails?buildbucketId=8936985836262846928

chromeos-installer-0.0.3-r3183: x86_64-pc-linux-gnu-clang++ -MMD -MF obj/installer/libcros_installer.inst_util.o.d -DNDEBUG -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/verity -Iobj/installer/libcros_installer.gen/include -Igen/include -I/var/tmp/portage/chromeos-base/chromeos-installer-0.0.3-r3183/work/chromeos-installer-0.0.3 -Wall -Wno-psabi -Wunused -Wno-unused-parameter -ggdb3 -fstack-protector-strong -Wformat=2 -fvisibility=internal -Wa,--noexecstack -Werror -DUSE_RTTI_FOR_TYPE_TAGS -Wno-c++11-extensions -Wno-unused-local-typedefs -DBASE_VER=395517 -pthread -I/usr/include/base-395517 -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss -I/usr/include/nspr -I/usr/include/dbus-1.0 -I/usr/lib64/dbus-1.0/include -I/usr/include/chromeos -fPIE -std=gnu++14 -DNDEBUG -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -O2 -pipe -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables  -c ../../../../../../tmp/portage/chromeos-base/chromeos-installer-0.0.3-r3183/work/chromeos-installer-0.0.3/installer/inst_util.cc -o obj/installer/libcros_installer.inst_util.o
chromeos-installer-0.0.3-r3183: In file included from ../../../../../../tmp/portage/chromeos-base/chromeos-installer-0.0.3-r3183/work/chromeos-installer-0.0.3/installer/inst_util.cc:13:
chromeos-installer-0.0.3-r3183: /usr/include/linux/fs.h:366:22: error: expected ';' after top level declarator
chromeos-installer-0.0.3-r3183: typedef int __bitwise __kernel_rwf_t;

This is because <linux/fs.h> expected __bitwise to be defined in <linux/types.h>, except <linux/types.h> is getting provided by /usr/include/linux/verity/linux/types.h, which is old and incompatible with the new /usr/include/linux/

I'm not entirely sure what to do with this. At the crux of it, it looks like verity really only needs to install a verity/dm-bht.h and verity/dm-bht-userspace.h, except that these headers have implicit dependencies on a bunch of kernel internals.

I'd *like* to make the verity package install much less garbage into /usr/include/. As a cheaper hack, I could probably "upgrade" some of the headers in verity/ such that they make good-enough replacements for the one package that seems to use its headers (chromeos-installer).

Thoughts? Suggestions?
 
Ah that's unfortunate. I would not object to update the headers a little bit, as long as our tests don't break.

I might need to take a closer look to figure out if we can install fewer things and use more system things. Mike, thoughts?
i think we're at the point where verity has been around long enough to not need to tie us so directly to the kernel.  iiuc, historically the code was structured this way because the code under verity/ was a direct copy of the kernel code.  that's no longer the case.  plus, we're talking about a mere ~1k loc for verity .c/.h files.

so my proposal would be:
- agree that keeping the kernelisms in the code base is not useful.
- delete all the hacks related to those.  that means converting all the kernel-like calls in our code to the corresponding C library code.
- delete all the kernel header copies/conflicts we have.

at this point, we have to ask ourselves who the target market for this project ?  do we want to keep it standalone so non-CrOS people can use it ?  do we really think there are that many people out there doing so ?  i'd point out this repo has seen like 3 CLs since 2013, one of which was fixing libc++ compatibility issues, and no outside contributions.  maybe it's a "stable" enough codebase that, for people outside of CrOS, they can just use this old copy.

so if we just accept that no one outside of CrOS cares for updates:
- lets merge this project into platform2
- convert the small amount of C code to C++
- switch from custom utility/library code to libchrome (e.g. all the hashing logic)

outside of the `verity` program, installer/ is the only user of the dm-bht library.  so converting this stuff to C++ shouldn't be a big deal.
I'm not familiar with much history of this particular repository. Most of that sounds good to me, with a special priority for things like "get rid of kernelisms".
To clarify on my use of "most": I'm not sure if it's worth a C++ rewrite. I wouldn't necessarily object, but I'm not volunteering to spend much time on it.
verity is already mostly C++ :).  the C code consists of two C files of mostly utility code.  my suggestion for going "all in" on C++ is so we can leverage the existing utility libraries rather than the copies we have under kernel/.
I agree with Mike that reality seems to have shown us that maybe, we don't need to keep this consumable by everyone.
we can def do the kernelism cleanups in the verity repo as-is and not make it depend on any C++ or platform2 migrations.  i was just taking the opportunity to think big :).
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/c480cc4023b11195e89974df17e11321901d9e2c

commit c480cc4023b11195e89974df17e11321901d9e2c
Author: Brian Norris <briannorris@chromium.org>
Date: Wed Aug 29 18:15:48 2018

verity: drop <linux/dm-bht.h>

It's a locally-provided header (not really in the <linux/...>
namespace); its external users use it under the <verity/...> namespace;
and it causes a QA package notice:

 * QA Notice: Symbolic link /usr/include/verity/linux/dm-bht.h points to /usr/include/dm-bht.h which does not exist.

So let's kill the symlink.

BUG= chromium:878440 
TEST=build chromeos-base/verity and chromeos-base/chromeos-installer

Change-Id: Ia7f126e157f0ee063a4eed462f333bfa9015de5b
Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1194386
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[modify] https://crrev.com/c480cc4023b11195e89974df17e11321901d9e2c/dm-bht-userspace.c
[delete] https://crrev.com/936245a6c19f36e58017f91fe2398f2d8ca45c9b/include/linux/dm-bht.h
[modify] https://crrev.com/c480cc4023b11195e89974df17e11321901d9e2c/dm-bht.c

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/936245a6c19f36e58017f91fe2398f2d8ca45c9b

commit 936245a6c19f36e58017f91fe2398f2d8ca45c9b
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Aug 29 18:15:46 2018

pull in standard linux/types.h directly

We only need to overlay a few of our own types, so pull in the
standard linux/types.h and overlay our stuff on top of it.

BUG= chromium:878440 
TEST=build works

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

[modify] https://crrev.com/936245a6c19f36e58017f91fe2398f2d8ca45c9b/include/linux/types.h

Project Member

Comment 14 by bugdroid1@chromium.org, Aug 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/bddbb0eb17b7833e4b2d2885005783e0e0567e6f

commit bddbb0eb17b7833e4b2d2885005783e0e0567e6f
Author: Mike Frysinger <vapier@chromium.org>
Date: Fri Aug 31 12:22:49 2018

purge linux/cpumask.h usage

We've hardcoded these values to "one cpu", so just inline it in the
single place that uses it.

BUG= chromium:878440 
TEST=build works

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

[delete] https://crrev.com/d04b19b9fe2f5a4e5a23c09a0b55e84ce7d1439c/include/linux/cpumask.h
[modify] https://crrev.com/bddbb0eb17b7833e4b2d2885005783e0e0567e6f/include/linux/types.h
[modify] https://crrev.com/bddbb0eb17b7833e4b2d2885005783e0e0567e6f/dm-bht.c
[modify] https://crrev.com/bddbb0eb17b7833e4b2d2885005783e0e0567e6f/dm-bht.h

Project Member

Comment 15 by bugdroid1@chromium.org, Aug 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/14335831d82440d7027d9c131daf68639eb99472

commit 14335831d82440d7027d9c131daf68639eb99472
Author: Mike Frysinger <vapier@chromium.org>
Date: Fri Aug 31 22:08:44 2018

purge linux/compiler.h usage

Turns out nothing in here actually uses definitions from this header,
so punting it is easy.

BUG= chromium:878440 
TEST=build works

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

[modify] https://crrev.com/14335831d82440d7027d9c131daf68639eb99472/include/linux/crypto.h
[modify] https://crrev.com/14335831d82440d7027d9c131daf68639eb99472/dm-bht.h
[delete] https://crrev.com/bddbb0eb17b7833e4b2d2885005783e0e0567e6f/include/linux/compiler.h

Project Member

Comment 16 by bugdroid1@chromium.org, Aug 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/36b0f69173495a7eb003147a96aef266dabf7564

commit 36b0f69173495a7eb003147a96aef266dabf7564
Author: Mike Frysinger <vapier@chromium.org>
Date: Fri Aug 31 22:08:44 2018

purge linux/err.h usage

Only used in one place, so inline it.

BUG= chromium:878440 
TEST=build works

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

[delete] https://crrev.com/14335831d82440d7027d9c131daf68639eb99472/include/linux/err.h
[modify] https://crrev.com/36b0f69173495a7eb003147a96aef266dabf7564/dm-bht.c

Project Member

Comment 17 by bugdroid1@chromium.org, Sep 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/6a1e0722330567e88742f191f5acb10804528553

commit 6a1e0722330567e88742f191f5acb10804528553
Author: Mike Frysinger <vapier@chromium.org>
Date: Sun Sep 02 01:33:50 2018

purge mempool code

We aren't using this anywhere, so drop it entirely.

BUG= chromium:878440 
TEST=build works

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

[delete] https://crrev.com/36b0f69173495a7eb003147a96aef266dabf7564/kernel/mempool.c
[delete] https://crrev.com/36b0f69173495a7eb003147a96aef266dabf7564/include/linux/mempool.h

Project Member

Comment 18 by bugdroid1@chromium.org, Sep 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/4c27ac6cef21c0b0e7cddbef1b7be32e91333267

commit 4c27ac6cef21c0b0e7cddbef1b7be32e91333267
Author: Mike Frysinger <vapier@chromium.org>
Date: Sun Sep 02 12:19:45 2018

purge linux/slab.h usage

Replace the kernel memory alloc funcs with the standard C library ones.

BUG= chromium:878440 
TEST=build works

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

[delete] https://crrev.com/6a1e0722330567e88742f191f5acb10804528553/include/linux/slab.h
[modify] https://crrev.com/4c27ac6cef21c0b0e7cddbef1b7be32e91333267/dm-bht.c
[delete] https://crrev.com/6a1e0722330567e88742f191f5acb10804528553/kernel/slab.c

Project Member

Comment 19 by bugdroid1@chromium.org, Sep 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vboot_reference/+/483fcf08acb9ad9bbedbb5f980314144260d5da4

commit 483fcf08acb9ad9bbedbb5f980314144260d5da4
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Sep 05 04:04:01 2018

add extern C markers to installed headers

Some of the headers have extern C markings already, so add to the
rest of the installed files so users don't have to.

BUG= chromium:878440 
TEST=build passes
BRANCH=none

Change-Id: I3edf56ca2235269803049207806a9f7eb4c664f2
Reviewed-on: https://chromium-review.googlesource.com/1201042
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>

[modify] https://crrev.com/483fcf08acb9ad9bbedbb5f980314144260d5da4/firmware/include/tpm1_tss_constants.h
[modify] https://crrev.com/483fcf08acb9ad9bbedbb5f980314144260d5da4/firmware/include/vboot_struct.h
[modify] https://crrev.com/483fcf08acb9ad9bbedbb5f980314144260d5da4/firmware/include/bmpblk_header.h
[modify] https://crrev.com/483fcf08acb9ad9bbedbb5f980314144260d5da4/firmware/include/tpm2_marshaling.h
[modify] https://crrev.com/483fcf08acb9ad9bbedbb5f980314144260d5da4/host/include/cgpt_params.h
[modify] https://crrev.com/483fcf08acb9ad9bbedbb5f980314144260d5da4/firmware/include/gpt.h
[modify] https://crrev.com/483fcf08acb9ad9bbedbb5f980314144260d5da4/host/include/vboot_host.h
[modify] https://crrev.com/483fcf08acb9ad9bbedbb5f980314144260d5da4/firmware/include/tpm2_tss_constants.h
[modify] https://crrev.com/483fcf08acb9ad9bbedbb5f980314144260d5da4/firmware/include/gbb_access.h
[modify] https://crrev.com/483fcf08acb9ad9bbedbb5f980314144260d5da4/firmware/include/gpt_misc.h
[modify] https://crrev.com/483fcf08acb9ad9bbedbb5f980314144260d5da4/firmware/2lib/include/2id.h
[modify] https://crrev.com/483fcf08acb9ad9bbedbb5f980314144260d5da4/firmware/include/vboot_api.h
[modify] https://crrev.com/483fcf08acb9ad9bbedbb5f980314144260d5da4/firmware/lib21/include/vb21_common.h
[modify] https://crrev.com/483fcf08acb9ad9bbedbb5f980314144260d5da4/firmware/lib21/include/vb21_struct.h
[modify] https://crrev.com/483fcf08acb9ad9bbedbb5f980314144260d5da4/host/include/openssl_compat.h

Project Member

Comment 20 by bugdroid1@chromium.org, Sep 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/39ff0dcebbf225b4f9cce2186fed9a276efa9100

commit 39ff0dcebbf225b4f9cce2186fed9a276efa9100
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Sep 05 04:04:21 2018

drop unused ruby script

No one uses ruby and this script hasn't been updated since the start
of the project.  The algos are a settled matter at this point.

BUG= chromium:878440 
TEST=build works

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

[delete] https://crrev.com/084d4d9ebb4eb26283bd80b7d7ff82888bfb770d/scripts/tree_alg.rb

Project Member

Comment 21 by bugdroid1@chromium.org, Sep 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/ab21acbd0d5afd664eb92f186c90088674d65a0d

commit ab21acbd0d5afd664eb92f186c90088674d65a0d
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Sep 05 04:04:00 2018

drop unused Digester class

Nothing refers to this file anywhere.  Delete it.

BUG= chromium:878440 
TEST=build works

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

[delete] https://crrev.com/0c6a4031bf89804c41037acbfe5e7cf29f7532de/digester.h

Project Member

Comment 22 by bugdroid1@chromium.org, Sep 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/0c6a4031bf89804c41037acbfe5e7cf29f7532de

commit 0c6a4031bf89804c41037acbfe5e7cf29f7532de
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Sep 05 04:03:59 2018

verity: clean up list of supported algos

Even though the help text says we support a lot of algos, they're all
a lie!  We only register md5/sha1/sha256 internally which means all
other values get rejected.

BUG= chromium:878440 
TEST=build works

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

[modify] https://crrev.com/0c6a4031bf89804c41037acbfe5e7cf29f7532de/verity_main.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Sep 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/084d4d9ebb4eb26283bd80b7d7ff82888bfb770d

commit 084d4d9ebb4eb26283bd80b7d7ff82888bfb770d
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Sep 05 04:04:14 2018

inline asm/unaligned.h usage

Only one module uses the funcs from this header, so inline that
single caller to avoid keeping this header around.

BUG= chromium:878440 
TEST=build works

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

[modify] https://crrev.com/084d4d9ebb4eb26283bd80b7d7ff82888bfb770d/kernel/sha1.c
[delete] https://crrev.com/ab21acbd0d5afd664eb92f186c90088674d65a0d/include/asm/unaligned.h

Project Member

Comment 24 by bugdroid1@chromium.org, Sep 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/c001c766ed178f745b2aa5e17d6a4b16141a1234

commit c001c766ed178f745b2aa5e17d6a4b16141a1234
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Sep 05 12:11:42 2018

punt unused crypto code

We don't actually compile the sha512 code, so drop it and related.

BUG= chromium:878440 
TEST=build works

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

[modify] https://crrev.com/c001c766ed178f745b2aa5e17d6a4b16141a1234/include/linux/cryptohash.h
[delete] https://crrev.com/39ff0dcebbf225b4f9cce2186fed9a276efa9100/kernel/sha512_generic.c
[delete] https://crrev.com/39ff0dcebbf225b4f9cce2186fed9a276efa9100/include/linux/percpu.h

Project Member

Comment 25 by bugdroid1@chromium.org, Sep 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/c07207b7839ec34a1a91b15dc44c78edbf11b11e

commit c07207b7839ec34a1a91b15dc44c78edbf11b11e
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Sep 05 23:11:22 2018

purge asm-generic/bitops/non-atomic.h usage

Nothing uses the helpers in this file, so drop it.

BUG= chromium:878440 
TEST=build works

Change-Id: Iee485ba78b3cbc86fbeb2acd19c28315db327f1b
Reviewed-on: https://chromium-review.googlesource.com/1201047
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[modify] https://crrev.com/c07207b7839ec34a1a91b15dc44c78edbf11b11e/include/linux/bitops.h
[delete] https://crrev.com/c001c766ed178f745b2aa5e17d6a4b16141a1234/include/asm-generic/bitops/non-atomic.h

Project Member

Comment 26 by bugdroid1@chromium.org, Sep 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/330147e003d3cf2cdfd4bebda5c67583308707ab

commit 330147e003d3cf2cdfd4bebda5c67583308707ab
Author: Mike Frysinger <vapier@chromium.org>
Date: Thu Sep 06 04:04:26 2018

move extern C markers to dm-bht headers

C code should mark its own headers as extern, so move that logic here.

BUG= chromium:878440 
TEST=build passes

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

[modify] https://crrev.com/330147e003d3cf2cdfd4bebda5c67583308707ab/dm-bht_unittest.cc
[modify] https://crrev.com/330147e003d3cf2cdfd4bebda5c67583308707ab/dm-bht-userspace.h
[modify] https://crrev.com/330147e003d3cf2cdfd4bebda5c67583308707ab/dm-bht.h
[modify] https://crrev.com/330147e003d3cf2cdfd4bebda5c67583308707ab/file_hasher.h

Project Member

Comment 27 by bugdroid1@chromium.org, Sep 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/89ab3bbce8798d9187813e2db3f5c70d9e58bba0

commit 89ab3bbce8798d9187813e2db3f5c70d9e58bba0
Author: Mike Frysinger <vapier@chromium.org>
Date: Sat Sep 08 13:49:58 2018

purge asm/atomic.h usage

We've never really guaranteed that this code works in a multithreaded
setup, and the only two users of this code are single threaded (`verity`
and `cros_installer`).  So simplify the atomic_t logic by inlining it
all and then dump the support code.

BUG= chromium:878440 
TEST=build works

Change-Id: I5fb95c5623b937e930dac00d37edcb7effdada64
Reviewed-on: https://chromium-review.googlesource.com/1201402
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/89ab3bbce8798d9187813e2db3f5c70d9e58bba0/dm-bht.h
[delete] https://crrev.com/330147e003d3cf2cdfd4bebda5c67583308707ab/kernel/atomic.c
[modify] https://crrev.com/89ab3bbce8798d9187813e2db3f5c70d9e58bba0/include/linux/types.h
[modify] https://crrev.com/89ab3bbce8798d9187813e2db3f5c70d9e58bba0/dm-bht.c
[modify] https://crrev.com/89ab3bbce8798d9187813e2db3f5c70d9e58bba0/dm-bht-userspace.c
[delete] https://crrev.com/330147e003d3cf2cdfd4bebda5c67583308707ab/include/asm/atomic.h

Project Member

Comment 28 by bugdroid1@chromium.org, Sep 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/bdbf8b1681700a99317eab2513edc86abd6556d0

commit bdbf8b1681700a99317eab2513edc86abd6556d0
Author: Mike Frysinger <vapier@chromium.org>
Date: Mon Sep 10 20:33:42 2018

purge linux/gfp.h usage

Inline or drop usage of this header so we can drop it.

BUG= chromium:878440 
TEST=build works

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

[modify] https://crrev.com/bdbf8b1681700a99317eab2513edc86abd6556d0/dm-bht-userspace.c
[modify] https://crrev.com/bdbf8b1681700a99317eab2513edc86abd6556d0/dm-bht.c
[delete] https://crrev.com/89ab3bbce8798d9187813e2db3f5c70d9e58bba0/include/linux/gfp.h

Project Member

Comment 29 by bugdroid1@chromium.org, Sep 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/ec14b916507882f5eccb564a3cca66c17014eccf

commit ec14b916507882f5eccb564a3cca66c17014eccf
Author: Mike Frysinger <vapier@chromium.org>
Date: Mon Sep 10 20:33:43 2018

purge linux/scatterlist.h usage

There are only 4 users of this scatterlist API:
- dm-bht.c: dm_bht_verify_block (and internal funcs only it calls)
- dm-bht-userspace.c: dm_bht_compute_hash
- crypto.c: crypto_hash_digest & crypto_hash_update

For crypto.c, nothing calls crypto_hash_digest.  So purge that.

For dm-bht.c, dm_bht_verify_block has long required that offset == 0.
This allows us to inline that offset down through dm_bht_compute_hash.
That leaves dm_bht_compute_hash which is processing a single buffer to
crypto_hash_update ...

For dm-bht-userspace.c, we see the same pattern.  dm_bht_compute_hash
always has an offset == 0, and passes a single buffer at a time down
to crypto_hash_update ...

That leaves us with crypto_hash_update.  Since it only cares about a
single buffer, rewrite the crypto API to accept the buffer directly
rather than going through the scatterlist structure.

At this point, nothing uses scatterlist, so drop it entirely.

BUG= chromium:878440 
TEST=build works

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

[delete] https://crrev.com/bdbf8b1681700a99317eab2513edc86abd6556d0/include/linux/scatterlist.h
[modify] https://crrev.com/ec14b916507882f5eccb564a3cca66c17014eccf/kernel/crypto.c
[delete] https://crrev.com/bdbf8b1681700a99317eab2513edc86abd6556d0/kernel/scatterlist.c
[modify] https://crrev.com/ec14b916507882f5eccb564a3cca66c17014eccf/dm-bht.c
[modify] https://crrev.com/ec14b916507882f5eccb564a3cca66c17014eccf/dm-bht-userspace.c
[modify] https://crrev.com/ec14b916507882f5eccb564a3cca66c17014eccf/include/linux/crypto.h

Project Member

Comment 30 by bugdroid1@chromium.org, Sep 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/16e1fa54bcebc58b4abe19f69f64f9d2acdfbe9e

commit 16e1fa54bcebc58b4abe19f69f64f9d2acdfbe9e
Author: Mike Frysinger <vapier@chromium.org>
Date: Tue Sep 11 00:17:50 2018

purge struct page usage

The only users of struct page treat it like a straight 4k buffer,
so inline the logic so it is just a 4k buffer.

BUG= chromium:878440 
TEST=build works

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

[modify] https://crrev.com/16e1fa54bcebc58b4abe19f69f64f9d2acdfbe9e/dm-bht-userspace.c
[modify] https://crrev.com/16e1fa54bcebc58b4abe19f69f64f9d2acdfbe9e/include/asm/page.h
[modify] https://crrev.com/16e1fa54bcebc58b4abe19f69f64f9d2acdfbe9e/dm-bht.c
[modify] https://crrev.com/16e1fa54bcebc58b4abe19f69f64f9d2acdfbe9e/dm-bht.h
[modify] https://crrev.com/16e1fa54bcebc58b4abe19f69f64f9d2acdfbe9e/dm-bht_unittest.cc

Project Member

Comment 33 by bugdroid1@chromium.org, Sep 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/3619f6218f73967552885d78b8c9f17741059486

commit 3619f6218f73967552885d78b8c9f17741059486
Author: Mike Frysinger <vapier@chromium.org>
Date: Tue Sep 18 13:42:12 2018

pull in standard linux/kernel.h directly

We only need to overlay a few of our own types, so pull in the
standard linux/kernel.h and overlay our stuff on top of it.

BUG= chromium:878440 
TEST=build works

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

[modify] https://crrev.com/3619f6218f73967552885d78b8c9f17741059486/include/linux/kernel.h
[modify] https://crrev.com/3619f6218f73967552885d78b8c9f17741059486/dm-bht.c

Project Member

Comment 34 by bugdroid1@chromium.org, Sep 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/dm-verity/+/e4d233306d0a67b2527fbaff03a9d912922a56cd

commit e4d233306d0a67b2527fbaff03a9d912922a56cd
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Sep 19 15:59:31 2018

update README to be markdown compliant

BUG= chromium:878440 
TEST=read it

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

[rename] https://crrev.com/e4d233306d0a67b2527fbaff03a9d912922a56cd/README.md

Blocking: 886953
Owner: vapier@chromium.org
Status: Fixed (was: Untriaged)
i've gotten through as much cleanup as i can at this point while keeping it in C.  so i'm going move it to platform2 where we can then do work to convert it to C++ and re-use existing base helpers for the hashing logic.

with the hashing logic removed, we should be able to purge the majority of the remaining kernel headers.

but i'll call this bug fixed as the kernel headers that are remaining don't generally conflict with /usr/include/linux/ anymore.
Thanks Mike!
Project Member

Comment 38 by bugdroid1@chromium.org, Sep 21

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

commit cb678e8fd290a35820bf9369ff36d2c1e5d5972c
Author: Mike Frysinger <vapier@chromium.org>
Date: Fri Sep 21 07:51:20 2018

installer: replace StartsWith/EndsWith to base versions

BUG= chromium:878440 
TEST=build passes

Change-Id: Icbec29de18bcdbd879d160188dd105d7c048946d
Reviewed-on: https://chromium-review.googlesource.com/1200805
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>

[modify] https://crrev.com/cb678e8fd290a35820bf9369ff36d2c1e5d5972c/installer/inst_util_test.cc
[modify] https://crrev.com/cb678e8fd290a35820bf9369ff36d2c1e5d5972c/installer/inst_util.cc
[modify] https://crrev.com/cb678e8fd290a35820bf9369ff36d2c1e5d5972c/installer/inst_util.h

Project Member

Comment 39 by bugdroid1@chromium.org, Oct 23

Labels: merge-merged-firmware-grunt-11031.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vboot_reference/+/6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4

commit 6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4
Author: Mike Frysinger <vapier@chromium.org>
Date: Tue Oct 23 22:33:06 2018

add extern C markers to installed headers

Some of the headers have extern C markings already, so add to the
rest of the installed files so users don't have to.

BUG= chromium:878440 
TEST=build passes
BRANCH=none

Change-Id: I3edf56ca2235269803049207806a9f7eb4c664f2
Reviewed-on: https://chromium-review.googlesource.com/1201042
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/1297017
Reviewed-by: Martin Roth <martinroth@chromium.org>
Tested-by: Martin Roth <martinroth@chromium.org>

[modify] https://crrev.com/6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4/firmware/include/tpm1_tss_constants.h
[modify] https://crrev.com/6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4/firmware/include/vboot_struct.h
[modify] https://crrev.com/6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4/firmware/include/bmpblk_header.h
[modify] https://crrev.com/6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4/firmware/include/tpm2_marshaling.h
[modify] https://crrev.com/6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4/host/include/cgpt_params.h
[modify] https://crrev.com/6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4/firmware/include/gpt.h
[modify] https://crrev.com/6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4/host/include/vboot_host.h
[modify] https://crrev.com/6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4/firmware/include/tpm2_tss_constants.h
[modify] https://crrev.com/6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4/firmware/include/gbb_access.h
[modify] https://crrev.com/6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4/firmware/include/gpt_misc.h
[modify] https://crrev.com/6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4/firmware/2lib/include/2id.h
[modify] https://crrev.com/6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4/firmware/include/vboot_api.h
[modify] https://crrev.com/6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4/firmware/lib21/include/vb21_common.h
[modify] https://crrev.com/6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4/firmware/lib21/include/vb21_struct.h
[modify] https://crrev.com/6e5920faa9bb41f9a99b5f6a4efa1d8ae1e99eb4/host/include/openssl_compat.h

Project Member

Comment 40 by bugdroid1@chromium.org, Dec 18

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

commit daf80917c9edbd0adcc90249bb009bbf48c85ee2
Author: Mike Frysinger <vapier@chromium.org>
Date: Tue Dec 18 12:28:55 2018

installer: replace custom StringPrintf implementation

In some cases we can use simple string concat operations.
In others, we can switch to base::FilePath.
For the few left, we can use base::StringPrintf.

BUG= chromium:878440 
TEST=build passes

Change-Id: I83b99edcf92e1aa3d0f7d52da93722858f465ae2
Reviewed-on: https://chromium-review.googlesource.com/1200806
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/daf80917c9edbd0adcc90249bb009bbf48c85ee2/installer/inst_util_test.cc
[modify] https://crrev.com/daf80917c9edbd0adcc90249bb009bbf48c85ee2/installer/cgpt_manager.cc
[modify] https://crrev.com/daf80917c9edbd0adcc90249bb009bbf48c85ee2/installer/chromeos_legacy.cc
[modify] https://crrev.com/daf80917c9edbd0adcc90249bb009bbf48c85ee2/installer/inst_util.cc
[modify] https://crrev.com/daf80917c9edbd0adcc90249bb009bbf48c85ee2/installer/inst_util.h
[modify] https://crrev.com/daf80917c9edbd0adcc90249bb009bbf48c85ee2/installer/chromeos_postinst.cc

Sign in to add a comment