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

Issue 6835032: CHROMIUM: verity: don't call page_address on unmapped highmem pages (Closed)

Created:
9 years, 6 months ago by Mandeep Singh Baines
Modified:
9 years, 5 months ago
CC:
chromium-os-reviews_chromium.org
Visibility:
Public.

Description

CHROMIUM: verity: don't call page_address on unmapped highmem pages This fixes a bug that is preventing ARM systems from booting with verity. We were calling page_address on what could potentially have been an unmapped highmem page. Fix is to pass the struct page to the crypto code and let crypto handle the kmap/kunmap. BUG=chromium-os:14089, chrome-os-partner:3287 TEST=Ran dm-verity.git unit tests. Ran platform_DMVerityCorruption on H/W. Verified that ARM now boots. Change-Id: Id2074052aab9eec8fa0aead39aded2b40cd19bad Signed-off-by: Mandeep Singh Baines <msb@chromium.org>; R=wad@chromium.org,olofj@chromium.org,taysom@chromium.org Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=76c7a42

Patch Set 1 #

Total comments: 9

Patch Set 2 : Fix. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -23 lines) Patch
M drivers/md/dm-bht.c View 1 8 chunks +15 lines, -21 lines 0 comments Download
M drivers/md/dm-verity.c View 1 chunk +1 line, -1 line 0 comments Download
M include/linux/dm-bht.h View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 10 (0 generated)
Mandeep Singh Baines
9 years, 6 months ago (2011-04-13 20:09:49 UTC) #1
Will Drewry
Thanks for tracking this down! A few questions, but looks good! http://codereview.chromium.org/6835032/diff/1/drivers/md/dm-bht.c File drivers/md/dm-bht.c (right): ...
9 years, 6 months ago (2011-04-13 20:19:30 UTC) #2
Paul T
See comment. http://codereview.chromium.org/6835032/diff/1/drivers/md/dm-bht.c File drivers/md/dm-bht.c (right): http://codereview.chromium.org/6835032/diff/1/drivers/md/dm-bht.c#newcode539 drivers/md/dm-bht.c:539: entry->nodes = page_address(node_page); This must assume the ...
9 years, 6 months ago (2011-04-13 20:47:55 UTC) #3
Mandeep Singh Baines
Fixed. PTAL. http://codereview.chromium.org/6835032/diff/1/drivers/md/dm-bht.c File drivers/md/dm-bht.c (right): http://codereview.chromium.org/6835032/diff/1/drivers/md/dm-bht.c#newcode539 drivers/md/dm-bht.c:539: entry->nodes = page_address(node_page); On 2011/04/13 20:19:30, Will ...
9 years, 6 months ago (2011-04-13 21:11:16 UTC) #4
Olof Johansson
Comments below. Mostly worried about hardcoded 0-offsets that can easily be computed (even if they ...
9 years, 6 months ago (2011-04-13 21:11:57 UTC) #5
Mandeep Singh Baines
On 2011/04/13 20:47:55, Paul T wrote: > See comment. > > http://codereview.chromium.org/6835032/diff/1/drivers/md/dm-bht.c > File drivers/md/dm-bht.c ...
9 years, 6 months ago (2011-04-13 21:14:03 UTC) #6
Mandeep Singh Baines
On 2011/04/13 21:11:57, Olof Johansson wrote: > Comments below. Mostly worried about hardcoded 0-offsets that ...
9 years, 6 months ago (2011-04-13 21:16:03 UTC) #7
Olof Johansson
On 2011/04/13 21:16:03, Mandeep Singh Baines wrote: > There are still a lot of assumptions ...
9 years, 6 months ago (2011-04-13 21:18:41 UTC) #8
taysom
LGTM On Wed, Apr 13, 2011 at 2:18 PM, <olofj@chromium.org> wrote: > On 2011/04/13 21:16:03, ...
9 years, 6 months ago (2011-04-13 21:22:53 UTC) #9
Will Drewry
9 years, 6 months ago (2011-04-13 21:37:27 UTC) #10
On 2011/04/13 21:18:41, Olof Johansson wrote:
> On 2011/04/13 21:16:03, Mandeep Singh Baines wrote:
> > There are still a lot of assumptions about block size == PAGE_SIZE. My plan
is
> > to clean these up in a later phase where I remove all these assumptions.
> 
> Given that there are current, ongoing, efforts to clean this up, this is OK
with
> me. If not, I would be wary -- the risk for someone not noticing the hardcoded
0
> and having to hunt a bug when changing unrelated code would be too large.
> 
> So, with that reservation: LGTM.

Ditto. LGTM.  Thanks for tracking this down!

Powered by Google App Engine
This is Rietveld 408576698