Messages in this thread | | | From | Arvind Sankar <> | Date | Sat, 18 Jul 2020 22:34:05 -0400 | Subject | Re: [PATCH] x86/idt: Make sure idt_table takes a whole page |
| |
On Sat, Jul 18, 2020 at 06:15:26PM -0700, hpa@zytor.com wrote: > On July 18, 2020 12:25:46 PM PDT, Andy Lutomirski <luto@amacapital.net> wrote: > > > >> On Jul 18, 2020, at 10:57 AM, hpa@zytor.com wrote: > >> > >> On July 9, 2020 3:33:55 AM PDT, Joerg Roedel <joro@8bytes.org> > >wrote: > >>> From: Joerg Roedel <jroedel@suse.de> > >>> > >>> On x86-32 the idt_table with 256 entries needs only 2048 bytes. It > >is > >>> page-aligned, but the end of the .bss..page_aligned section is not > >>> guaranteed to be page-aligned. > >>> > >>> As a result, symbols from other .bss sections may end up on the same > >>> 4k page as the idt_table, and will accidentially get mapped > >read-only > >>> during boot, causing unexpected page-faults when the kernel writes > >to > >>> them. > >>> > >>> Avoid this by making the idt_table 4kb in size even on x86-32. On > >>> x86-64 the idt_table is already 4kb large, so nothing changes there. > >>> > >>> Fixes: 3e77abda65b1c ("x86/idt: Consolidate idt functionality") > >>> Signed-off-by: Joerg Roedel <jroedel@suse.de> > >> > >> NAK... this isn't the right way to fix this and just really kicks the > >can down the road. The reason is that you aren't fixing the module that > >actually has a problem. > >> > >> The Right Way[TM] is to figure out which module(s) lack the proper > >alignment for this section. A script using objdump -h or readelf -SW > >running over the .o files looking for alignment less than 2**12 should > >spot the modules that are missing the proper .balign directives. > > > >I don’t see the problem. If we are going to treat an object as though > >it’s 4096 bytes, making C think it’s 4096 bytes seems entirely > >reasonable to me. > > > >> -- > > > >> Sent from my Android device with K-9 Mail. Please excuse my brevity. > > It isn't the object, it is its alignment. You can have an object > page-aligned so it doesn't cross page boundaries. > > The bigger issue, though, is that this means there are other object > files which don't have the correct alignment directives, which means > that this error can crop up again at any point. The really important > thing here is that we fix the real problem. -- > Sent from my Android device with K-9 Mail. Please excuse my brevity.
To repeat the commit message, the problem is not misaligned bss..page_aligned objects, but symbols in _other_ bss sections, which can get allocated in the last page of bss..page_aligned, because its end isn't page-aligned (maybe it should be?)
bss..page_aligned objects are unlikely to be misaligned, because its used in C via a macro that includes the alignment attribute, and its only use in x86 assembly is in head_{32,64}.S which have correct alignment.
Given that this IDT's page is actually going to be mapped with different page protections, it seems like allocating the full page isn't unreasonable.
| |