Fixing the bug of CGPT when primary entry table is invalid.

Fixing the bug of CGPT when primary entry table is invalid.

http://code.google.com/p/chromium-os/issues/detail?id=9279
This issue disclosed a bug of cgpt. The bug comes from the 'show' command always
reads the primary entry table when '-i partition' is specified. I added an
ANY_VALID constant for GetEntry to automatically select valid entry table.

Also fixed the bugs in cmd_boot.c and cmd_find.c. In cmd_add.c, stop user to
continue if any header/entry table is invalid.

Also fixed the bug that untrusted header size could cause segmentation failure.

Hungte, this is FYI. But welcome to do review.

BUG=chromium-os:9279
TEST=RUNTESTS=1 emerge-x86-generic vboot_reference
     Manually tested:
       cgpt show /tmp/test -i 1 -b
       cgpt show /tmp/test
       cgpt add /tmp/test -i 1 -l TEST
       cgpt find /tmp/test -l STATE
       cgpt boot /tmp/test -i 1

Change-Id: Iaba9c635754096a82b3ec74634af184362d4e264
Change-Id: I6f3e87e3998457676e3388d2a6ed36c0564796d8

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=2b23c02

[Hung-Te, 2010-11-17 10:37:04]

looks fine for me, but since I'm not really familiar with the design of cgpt,
please wait for Bill's confirm for this CL.

http://codereview.chromium.org/5115002/diff/1/cgpt/cmd_add.c
File cgpt/cmd_add.c (right):

http://codereview.chromium.org/5115002/diff/1/cgpt/cmd_add.c#newcode201
cgpt/cmd_add.c:201: if ((drive.gpt.valid_headers != MASK_BOTH) ||
it looks a little weird for me to compare a variable with "mask" by bit-wise
compare.

I'd suggest having a special value like CGPT_INDEX_BOTH=3 then
 (drive.gpt.valid.headers & MASK_BOTH) == CGPT_INDEX_BOTH,
or at least (drive.gpt.valid.headers & MASK_BOTH) == MASK_BOTH).

[Bill Richardson, 2010-11-17 18:06:24]

LGTM.

http://codereview.chromium.org/5115002/diff/1/cgpt/cgpt_common.c
File cgpt/cgpt_common.c (right):

http://codereview.chromium.org/5115002/diff/1/cgpt/cgpt_common.c#newcode486
cgpt/cgpt_common.c:486: require(ANY_VALID);
'require' is just 'assert' that doesn't compile away. You don't need to assert a
constant.

[Louis, 2010-11-18 01:51:28]

Thanks for review.

http://codereview.chromium.org/5115002/diff/1/cgpt/cgpt_common.c
File cgpt/cgpt_common.c (right):

http://codereview.chromium.org/5115002/diff/1/cgpt/cgpt_common.c#newcode486
cgpt/cgpt_common.c:486: require(ANY_VALID);
This is a typo. Should be "require(secondary==ANY_VALID);

Thank for the catch.
On 2010/11/17 18:06:24, Bill Richardson wrote:
> 'require' is just 'assert' that doesn't compile away. You don't need to assert
a
> constant.

http://codereview.chromium.org/5115002/diff/1/cgpt/cmd_add.c
File cgpt/cmd_add.c (right):

http://codereview.chromium.org/5115002/diff/1/cgpt/cmd_add.c#newcode201
cgpt/cmd_add.c:201: if ((drive.gpt.valid_headers != MASK_BOTH) ||
Good catch. Modified as:
  (valid_header & MASK_BOTH) != MASK_BOTH

Thanks.
On 2010/11/17 10:37:04, Hung-Te wrote:
> it looks a little weird for me to compare a variable with "mask" by bit-wise
> compare.
> 
> I'd suggest having a special value like CGPT_INDEX_BOTH=3 then
>  (drive.gpt.valid.headers & MASK_BOTH) == CGPT_INDEX_BOTH,
> or at least (drive.gpt.valid.headers & MASK_BOTH) == MASK_BOTH).