I don't see the problem here TBH. There's no issue, security or
otherwise, with coreboot having fmap regions within the IFD BIOS
region that are not contiguous. Alignment requirements (eg, 64k) for
some regions practically guarantee this.

ifdtool complained here because a manually generated flashmap had a
BIOS region that wasn't top-aligned with the IFD one.

On Thu, Jun 24, 2021 at 7:37 AM James Feeney <ja...@nurealm.net> wrote:
>
> On 6/24/21 12:48 AM, James Feeney wrote:
> > On 6/23/21 8:59 PM, James Feeney wrote:
> >> On 6/23/21 6:51 PM, Matt DeVillier wrote:
> >>> On Wed, Jun 23, 2021 at 4:36 PM James Feeney <ja...@nurealm.net> wrote:
> >>>>
> >>>> On 6/23/21 1:40 PM, Matt DeVillier wrote:
> >>>>> I'm an idiot and made a stupid math error when I calculated the size
> >>>>> of the SI_BIOS region. 0xF7F000 - 0x1000 is 0xF7E000, not 0xF6F000.
> >>>>>
> >>>>> https://review.coreboot.org/c/coreboot/+/55815 fixes it
> >>>>>
> >>>>
> >>>> Ha!  I was a little suspicious of the suggestive pattern, 0xF7F000 - 
> >>>> 0x10000 = 0xF6F000 vs 0xF7F000 - 0x1000 = 0xF7E000.  But, I missed the 
> >>>> hard-coded "SI_BIOS@0x1000 0xf6f000".
> >>>>
> >>>> So, I see src/mainboard/google/octopus/default.fmd, specifically, is 
> >>>> determining here.
> >>>>
> >>>> Still, many questions from those of us less familiar.  What is the 
> >>>> meaning of "SI"?  Does "SI_BIOS" have the same usage as "BIOS"?  
> >>>> Coreboot "BIOS" is not the same thing as original "BIOS".  
> >>>> <rant>...</rant>
> >>>
> >>> coreboot's FMAP is a more finely grained map of the firmware image
> >>> than the IFD's, which is necessary for coreboot (and vboot, and other
> >>> utils) to locate things in the different regions.
> >>>
> >>> I'm not sure where the SI_BIOS nomenclature comes from, it's a Google
> >>> thing I think so one of those folks from the old days would have to
> >>> chime in. On older boards it did/does encapsulate all of the coreboot
> >>> FMAP regions which reside inside the IFD's BIOS region.
> >>>>
> >>>> Are there any "official" specifications for coreboot fmap format?
> >>>
> >>> https://doc.coreboot.org/lib/flashmap.html
> >>>
> >>>> Wish me luck.  I should have a functional bootrom image now!
> >>>
> >>> well, it would have been functional before, as I've apparently been
> >>> using improperly sized images for quite some time :)
> >>
> >> https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+/255031/11/util/cbfstool/README.fmaptool
> >>
> >> "... gaps are allowed ..."
> >>
> >> So then, your images are actually, technically, "proper", even though some 
> >> rom space was wasted.
> >>
> >> But, this element of the specification strikes me as a waking big security 
> >> problem waiting to happen.
> >>
> >> I don't see any substantive reason for the Flashmap Descriptor 
> >> specification to allow gaps, and, as we have seen, allowing gaps can also 
> >> lead to inadvertant errors being introduced into the resulting flashmap 
> >> descriptor files.
> >>
> >> I believe, instead, that gaps should specifically be *prohibited* in the 
> >> Flashmap Descriptor specification.
> >>
> >> I suggest that the question of allowing gaps in the flashmap descriptor 
> >> should receive wider discussion, particularly as having - I would think 
> >> obvious - security implications, if not simply to avoid inadvertant errors.
> >>
> >>
> >> James
> >>
> >
> > Hmm - on second thought, I suppose that there are *always* going to be 
> > "gaps", since the rom is never "full", and the actual space taken by some 
> > random mix of files will not be known in advance.
> >
> > Maybe the only alternative, simply for the sake of consistency and error 
> > checking, is to require and define certain "contiguous" regions of rom, 
> > within which gaps would not be allowed.  That way, errors would be 
> > disclosed, while gaps might be "forced" into defined regions, which could 
> > be specifically "locked", if desired.
> >
> > Other people have thought about this?
> >
> >
> > James
>
> Or, a simpler idea that could be useful - cbfstool layout could just 
> autogenerate "GAP" regions in the output, to make these more apparent.
>
>
> James
_______________________________________________
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org

Reply via email to