[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 3/8] x86/traps: Avoid OoB accesses to print the data selectors
On 11.03.2025 22:10, Andrew Cooper wrote: > _show_registers() prints the data selectors from struct cpu_user_regs, but > these fields are sometimes out-of-bounds. See commit 6065a05adf15 > ("x86/traps: 'Fix' safety of read_registers() in #DF path"). > > There are 3 callers of _show_registers(): > > 1. vcpu_show_registers(), which always operates on a scheduled-out vCPU, > where v->arch.user_regs (or aux_regs on the stack) is always in-bounds. > > 2. show_registers() where regs is always an on-stack frame. regs is copied > into a local variable first (which is an OoB read for constructs such as > WARN()), before being modified (so no OoB write). > > 3. do_double_fault(), where regs is adjacent to the stack guard page, and > written into directly. This is an out of bounds read and write, with a > bodge to avoid the writes hitting the guard page. > > Include the data segment selectors in struct extra_state, and use those fields > instead of the fields in regs. This resolves the OoB write on the #DF path. > > Resolve the OoB read in show_registers() by doing a partial memcpy() rather > than full structure copy. This is temporary until we've finished untangling > the vm86 fields fully. > > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> > @@ -124,17 +128,23 @@ static void _show_registers( > state->fsb, state->gsb, state->gss); > printk("ds: %04x es: %04x fs: %04x gs: %04x " > "ss: %04x cs: %04x\n", > - regs->ds, regs->es, regs->fs, > - regs->gs, regs->ss, regs->cs); > + state->ds, state->es, state->fs, > + state->gs, regs->ss, regs->cs); > } > > void show_registers(const struct cpu_user_regs *regs) > { > - struct cpu_user_regs fault_regs = *regs; > + struct cpu_user_regs fault_regs; > struct extra_state fault_state; > enum context context; > struct vcpu *v = system_state >= SYS_STATE_smp_boot ? current : NULL; > > + /* > + * Don't read beyond the end of the hardware frame. It is out of bounds > + * for WARN()/etc. > + */ > + memcpy(&fault_regs, regs, offsetof(struct cpu_user_regs, es)); I don't like this (especially the assumption on es being special, much like e.g. get_stack_bottom() also does) very much, but I hope this is going to disappear at some point anyway. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |