Luca Dariz, le mar. 28 juin 2022 12:10:48 +0200, a ecrit:
> * i386/i386/pcb.c:
> - increase alignment of pcb cache to 16
> - ensure the stack is properly aligned when switching ktss
> * i386/i386/thread.h:
> - add padding tomake iss field end aligned to 16 bytes
> * i386/i386/trap.c:
> - ensure the state we get after the trap points to the correct place
> in the pcb structure
>
> When handling exceptions from IA-32e compatibility mode in user space,
> on a 64-bit kernel, the exception stack where error info is pushed
> needs to be aligned to 16 bytes (see Intel System Programming guide,
> $6.14.2)
>
> The exception stack frame is set in the middle of pcb->iss, but it's not
> always
> 16-byte aligned; to make sure it is, we increase the alignment of the
> pcb cache and add a padding field in the pcb structure.
>
> This issue resulted in a general protection failure due to CS being
> corrupted after a page fault. The corruption was happening when the
> exception stack frame was not properly aligned and a page fault
> happened; the error info was then pushed after re-aligning the stack,
> so the value of eflags was actually written in CS place and other
> fields were shifted too.
>
> It also makes sense to ensure this by adding two assertions, although
> these were primarly useful during debug.
>
> Signed-off-by: Luca Dariz <l...@orpolo.org>
Applied, thanks!
> ---
> i386/i386/pcb.c | 10 +++++++++-
> i386/i386/thread.h | 3 +++
> i386/i386/trap.c | 4 ++++
> 3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/i386/i386/pcb.c b/i386/i386/pcb.c
> index 6e215b3e..4a6cbdb0 100644
> --- a/i386/i386/pcb.c
> +++ b/i386/i386/pcb.c
> @@ -147,6 +147,9 @@ void switch_ktss(pcb_t pcb)
> pcb_stack_top = (pcb->iss.efl & EFL_VM)
> ? (long) (&pcb->iss + 1)
> : (long) (&pcb->iss.v86_segs);
> +#ifdef __x86_64__
> + assert((pcb_stack_top & 0xF) == 0);
> +#endif
>
> #ifdef MACH_RING1
> /* No IO mask here */
> @@ -375,7 +378,12 @@ thread_t switch_context(
>
> void pcb_module_init(void)
> {
> - kmem_cache_init(&pcb_cache, "pcb", sizeof(struct pcb), 0,
> + kmem_cache_init(&pcb_cache, "pcb", sizeof(struct pcb),
> +#ifdef __x86_64__
> + 16,
> +#else
> + 0,
> +#endif
> NULL, 0);
>
> fpu_module_init();
> diff --git a/i386/i386/thread.h b/i386/i386/thread.h
> index 4a9c1987..cb317bee 100644
> --- a/i386/i386/thread.h
> +++ b/i386/i386/thread.h
> @@ -200,6 +200,9 @@ struct i386_machine_state {
>
> typedef struct pcb {
> struct i386_interrupt_state iis[2]; /* interrupt and NMI */
> +#ifdef __x86_64__
> + unsigned long pad; /* ensure exception stack is aligned to 16 */
> +#endif
> struct i386_saved_state iss;
> struct i386_machine_state ims;
> decl_simple_lock_data(, lock)
> diff --git a/i386/i386/trap.c b/i386/i386/trap.c
> index 4f8612bc..23cb9f17 100644
> --- a/i386/i386/trap.c
> +++ b/i386/i386/trap.c
> @@ -361,6 +361,10 @@ int user_trap(struct i386_saved_state *regs)
> int type;
> thread_t thread = current_thread();
>
> +#ifdef __x86_64__
> + assert(regs == &thread->pcb->iss);
> +#endif
> +
> type = regs->trapno;
> code = 0;
> subcode = 0;
> --
> 2.30.2
>
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
