Re: [PATCH v2 2/3] x86: fix setup of brk area

[Juergen Gross]

On 29.06.22 19:14, Josh Poimboeuf wrote:
> Hi Juergen,
>
> It helps to actually Cc the person who broke it ;-)
>
> On Thu, Jun 23, 2022 at 11:46:07AM +0200, Juergen Gross wrote:
> > Commit e32683c6f7d2 ("x86/mm: Fix RESERVE_BRK() for older binutils")
> > put the brk area into the .bss..brk section (placed directly behind
> > .bss),
>
> Hm? It didn't actually do that.
>
> For individual translation units, it did rename the section from
> ".brk_reservation" to ".bss..brk".  But then during linking it's still
> placed in .brk in vmlinux, just like before.

Sorry, I misread the patch commit message and was fooled by the fact that
bisection clearly pointed at this patch to have introduced the problem.

I only discovered later that the main issue was the added "NOLOAD"
attribute.

> > causing it not to be cleared initially. As the brk area is used
> > to allocate early page tables, these might contain garbage in not
> > explicitly written entries.
> >
> > This is especially a problem for Xen PV guests, as the hypervisor will
> > validate page tables (check for writable page tables and hypervisor
> > private bits) before accepting them to be used. There have been reports
> > of early crashes of PV guests due to illegal page table contents.
> >
> > Fix that by letting clear_bss() clear the brk area, too.
>
> While it does make sense to clear the brk area, I don't understand how
> my patch broke this.  How was it getting cleared before?

It seemed to have worked by chance. The Xen hypervisor is clearing all
alloc-only sections when loading a kernel (this will "fix" the dom0
case reliably together with patch 3 of this series).

Grub might do the clearing, too (for the PV domU case), but I haven't
verified that by code inspection.

I'll drop the "Fixes:" tag.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature