On Wed, Feb 08, 2017 at 09:42:34AM -0600, Daniel Drake wrote:
> On Wed, Feb 8, 2017 at 8:22 AM, Peter Jones <pjones@xxxxxxxxxx> wrote:
> > The machine you've got does seem to be presenting a valid (if poorly
> > considered) table, but I don't quite think this is the right approach.
>
> I don't think it is fully valid. Section 22.3 of the UEFI spec says:
> "The ESRT shall be stored in memory of type EfiBootServicesData"
>
> > The first hunk of your patch is fine, but the test on the result of
> > efi_mem_desc_lookup() is more or less our check for "might reading this
> > table reboot the system or worse". You're demoting it to a warning
> > while still /treating/ it as an error, so at least it remains safe,
> > but the when we get to that point, they've (plausibly) given us data
> > that could cause horrible behavior, and that is a thing worth logging an
> > error about.
>
> Although from the user's perspective (thinking about graphical
> experience) it's a bit unfortunate to have early boot code like this
> print a cryptic error on-screen, when the system is otherwise
> perfectly usable and they might not even be interested in the ESRT's
> functionality.
Fair enough.
> > It would be better to /check/ if it straddles multiple contiguous
> > segments which both have reasonable access modes and print an error if
> > there are gaps. I'm still in favor of printing a warning to the log if
> > it straddles them and there /aren't/ gaps, because we're *going* to see
> > other weird bugs from any code base that decides it's stitching config
> > tables together from separate allocations. It's not a fundamentally
> > reasonable thing to do, and as a lens it shows us some code with a
> > really worrisome internal structure.
>
> If I'm following your suggestion correctly I think it would not affect
> the system I have here, where the ESRT sits entirely within a gap in
> the memory map, without touching the allocations on either side.
Oh! Then I've misunderstood your commit message. You said:
> Out of curiosity I looked closer at the Weibu F3C. There is no entry
> in the UEFI-provided memory map which corresponds to the ESRT pointer,
> but hacking the code to map it anyway, the ESRT does appear to be
> valid with 2 entries.
And I read "valid with 2 entries" to mean it straddles across two memory
map entries, but what you're saying is it has two entries in the ESRT.
So the machine is buggy, but you're right.
Acked-by: Peter Jones <pjones@xxxxxxxxxx>
--
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
