On 22 April 2016 at 19:25, Mark Rutland <mark.rutland@xxxxxxx> wrote:
> On Fri, Apr 22, 2016 at 07:20:38PM +0200, Ard Biesheuvel wrote:
>> On 22 April 2016 at 19:01, Mark Rutland <mark.rutland@xxxxxxx> wrote:
>> > On Fri, Apr 22, 2016 at 06:48:08PM +0200, Ard Biesheuvel wrote:
>
>> >> +static struct ptdump_info efi_ptdump_info = {
>> >> + .mm = &efi_mm,
>> >> + .markers = efi_addr_markers,
>> >> + .base_addr = 0,
>> >> + .max_addr = SZ_1G,
>> >> +};
>> >
>> > I see that max_addr isn't used for arm64, and for ARM it's only used in
>> > one place. It doesn't seem great to have that on arm64 given it's
>> > unused.
>> >
>> > Do we actually need max_addr? Is there any reason not to always dump
>> > whole tables?
>> >
>> > I guess you're trying to avoid dumping the kernel VA range on 32-bit?
>>
>> Indeed. On ARM, the efi_page_tables dumps its copy of
>> kernel_page_tables. If that is OK (the information could potentially
>> be useful, I suppose) we can drop the max
>
> I'd argue it's always worth dumping the full tables (dropipng the max).
>
> For instance, we could have some bug that leaves the kernel VA range in
> the EFI tables inconsistent with what we expect/require, and having that
> exposed would make that clear.
>
Well, the problem here is that we only sync the EFI page tables with
the kernel page tables in switch_mm() (if they turn out to be out of
sync). This means we should either sync them in this code path as
well, or remove the kernel VA range from the output, since we'll be
dumping known stale data otherwise.
> Regardless, arbitrarily limiting the VA range of the arm64 dump doesn't
> seem great.
>
True. [0 .. TASK_SIZE) looks like a useful and not arbitrary range to
limit the EFI page table output to. Unfortunately, TASK_SIZE resolves
to a value that depends on TIF_32BIT, so will still need some
preprocessor conditional to select between TASK_SIZE_64 (on arm64) and
TASK_SIZE (on ARM)
--
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
