[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 1/6] libelf: rewrite symtab/strtab loading for Dom0
El 22/01/16 a les 9.11, Jan Beulich ha escrit: >>>> On 21.01.16 at 18:55, <roger.pau@xxxxxxxxxx> wrote: >> El 21/01/16 a les 18.29, Ian Jackson ha escrit: >>> Roger Pau Monne writes ("[PATCH v4 1/6] libelf: rewrite symtab/strtab >> loading for Dom0"): >>>> Current implementation of elf_load_bsdsyms is broken when loading inside of >>>> a HVM guest, because it assumes elf_memcpy_safe is able to write into guest >>>> memory space, which it is not. >>>> >>>> Take the oportunity to do some cleanup and properly document how >>>> elf_{parse/load}_bsdsyms works. The new implementation uses elf_load_image >>>> when dealing with data that needs to be copied to the guest memory space. >>>> Also reduce the number of section headers copied to the minimum necessary. >>> ... >>>> #define elf_hdr_elm(_elf, _hdr, _elm, _val) \ >>>> do { \ >>>> if ( elf_64bit(_elf) ) \ >>>> - elf_store_field(_elf, _hdr, e64._elm, _val); \ >>>> + (_hdr).e64._elm = _val; \ >>> >>> This seems to bypass the safe store mechanism which was introduced to >>> fix XSA-55. >> >> This macro is only used to store fields inside of a structure that's >> allocated on the stack, and it doesn't involve any kind of pointer >> magic/arithmetic. The way it was used previously in this function indeed >> required the use of the _safe mechanism in order to prevent writing into >> arbitrary memory places, because we were actually modifying guest memory >> IIRC. >> >> I could restore the previous behaviour, but that would mean adding some >> handlers in order to access the structure. Since this is only used for >> Dom0, which already makes use of the elf_memcpy_unchecked function as >> called by elf_store_val which is in turn called from elf_store_field, so >> it's not like we were protected previously anyway. > > If this is indeed Dom0-only code, could (and perhaps should) it be > guarded suitably by an #ifdef to make obvious it's not used for > DomU loading, and hence not security sensitive? From looking at > the call sites of elf_{parse,load}_bsdsyms() I can't, btw., tell that > this is Dom0-only ... You are completely right, TBH I'm not sure what's going on with this. xc_dom_elfloader.c has it's own functions to load the strtab/symtab, but it's also calling elf_load_binary which, AFAICT, will call elf_load_bsdsyms. Am I missing something, or are we loading the symtab/strtab twice from libxc? Roger. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |