[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] x86emul: don't call ->read_segment() with x86_seg_none
On 06.08.2024 20:24, Stefano Stabellini wrote: > On Mon, 5 Aug 2024, Jan Beulich wrote: >> LAR, LSL, VERR, and VERW emulation involve calling protmode_load_seg() >> with x86_seg_none. The fuzzer's read_segment() hook function has an >> assertion which triggers in this case. Calling the hook function, >> however, makes little sense for those insns, as there's no data to >> retrieve. Instead zero-filling the output structure is what properly >> corresponds to those insns being invoked with a NUL selector. >> >> Fixes: 06a3b8cd7ad2 ("x86emul: support LAR/LSL/VERR/VERW") >> Oss-fuzz: 70918 >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> > > Looking at oss-fuzz's report and at this patch I think it is correct > > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx> S-o-b? > That said, there are a few other places where read_segment is called > without any checks for seg being x86_seg_none. The hvm implementation of > read_segment (hvmemul_read_segment) seems to return error if > x86_seg_none is passed as an argument, but there is no return error > checks any time we call ops->read_segment in x86_emulate.c. There is a pretty limited number of cases where x86_seg_none is used. For example, state->ea.mem.seg cannot hold this value. realmode_load_seg() also cannot be passed this value. We could add assertions to that effect, yet that can get unwieldy as further x86_seg_* enumerators are added (see "x86: introduce x86_seg_sys"; I expect we'll need at least one more when adding VMX/SVM insn emulation, where physical addresses are used as insn operands). > It seems that there might still be an issue? In my auditing I didn't spot any. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |