Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(34)

Issue 6811030: verity: remove the depth parameter from bht_create (Closed)

Created:
9 years, 8 months ago by Mandeep Singh Baines
Modified:
9 years, 7 months ago
Reviewers:
Paul T, ups, Will Drewry
CC:
chromium-os-reviews_chromium.org
Visibility:
Public.

Description

verity: remove the depth parameter from bht_create We want to only support regular tries with a single root hash block. BUG=chromium-os9752 TEST=Ran dm-verity.git unit tests. Ran platform_DMVerityCorruption on H/W. Change-Id: I49a8b74b8343c4cb5aa871a81b45f06025fe1011 R=wad@chromium.org,taysom@chromium.org,ups@chromium.org Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=31b3605

Patch Set 1 #

Total comments: 8

Patch Set 2 : Fix per review. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+39 lines, -80 lines) Patch
M dm-bht.h View 1 chunk +0 lines, -1 line 0 comments Download
M dm-bht.c View 1 4 chunks +7 lines, -16 lines 0 comments Download
M dm-bht_unittest.cc View 1 12 chunks +19 lines, -46 lines 2 comments Download
M file_hasher.h View 2 chunks +0 lines, -2 lines 0 comments Download
M file_hasher.cc View 3 chunks +3 lines, -6 lines 0 comments Download
M verity_main.cc View 5 chunks +10 lines, -9 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
Mandeep Singh Baines
9 years, 8 months ago (2011-04-07 21:39:08 UTC) #1
Paul T
LGTM http://codereview.chromium.org/6811030/diff/1/file_hasher.cc File file_hasher.cc (right): http://codereview.chromium.org/6811030/diff/1/file_hasher.cc#newcode121 file_hasher.cc:121: root_end, hash_start, alg_, digest); Do you need to ...
9 years, 8 months ago (2011-04-07 21:54:33 UTC) #2
Will Drewry
Looks pretty good to me. A couple of questions. I'm also curious about what happens ...
9 years, 8 months ago (2011-04-07 22:45:17 UTC) #3
Mandeep Singh Baines
Fixed. PTAL. We shouldn't see any wierdness. All users of verity have been converted to ...
9 years, 8 months ago (2011-04-08 14:28:40 UTC) #4
Mandeep Singh Baines
ping
9 years, 7 months ago (2011-04-11 17:23:23 UTC) #5
Paul T
LGTM
9 years, 7 months ago (2011-04-11 17:58:10 UTC) #6
Will Drewry
9 years, 7 months ago (2011-04-12 03:12:32 UTC) #7
LGTM

thanks!

http://codereview.chromium.org/6811030/diff/6001/dm-bht_unittest.cc
File dm-bht_unittest.cc (right):

http://codereview.chromium.org/6811030/diff/6001/dm-bht_unittest.cc#newcode182
dm-bht_unittest.cc:182: TEST_F(MemoryBhtTest, CreateThenVerifySingleLevel) {
Nit: SingleBlock :) 32 blocks will all end up hashed into one block.  It'd be
worth while to do at least 129 blocks I'd think.  But that could be
yet-another-test in a new CL since this is worthwhile too.

http://codereview.chromium.org/6811030/diff/6001/dm-bht_unittest.cc#newcode186
dm-bht_unittest.cc:186:
"2d3a43008286f56536fa24dcdbf14d342f0548827e374210415c7be0b610d2ba";

FWIW, I did manually cross check this hash:
$ rm /tmp/level.1.tmp; i=0; while [[ $i -lt 32 ]]; do head -c 4096 /dev/zero
|openssl sha -sha256 -binary >> /tmp/level.1.tmp; i=$((i+1)); done; head -c 4096
/dev/zero > /tmp/level.1;
# Pad it out to 4k
$ dd if=/tmp/level.1.tmp of=/tmp/level.1 conv=notrunc
# check the hash
$ openssl sha -sha256 /tmp/level.1
SHA256(/tmp/level.1)=
2d3a43008286f56536fa24dcdbf14d342f0548827e374210415c7be0b610d2ba

Once the trie changes are complete - I think it'd be worthwhile to manually
generate a range of expected roots for these (rather than just taking the new
output of verity).  I dunno if you've been doing that, but I know that's how I
tend to do it and only manually check intermittenly.

Powered by Google App Engine
This is Rietveld 408576698