Xen project Mailing List

Re: [Xen-devel] [PATCH] VT-d: improve RMRR validity checking

Hello Weidong, vt-d can have 2 goals: 1 security 2 passthrough a) I think (1) should be default for xen, but it requires that there are no bios problems with at least DRHD, RMRR could perhaps be worked around if i understand correctly. - When (1) security works, (2) passthrough will also work. - When this fails there are 2 ways to fail: - warn about problem and disable vt-d completely (your default with iommu=1) - panic (your option iommu=force) b) If there are problems with DRHD (and/or RMRR) security isn't a goal that can be met anymore, so only (2) passthrough remains. - provide an option to workaround bios options (your option iommu=workaround_bogus_bios). Comments: a) Could be discussed if panic should be default instead of disabling iommu or not, although there seem to be a lot of broken bioses, so that would lead to a lot of machines not booting. b) I think it would be best for the option "iommu=workaround_bogus_bios" to do a really best effort, and try to get as much of vt-d working to support passthrough (full security can't be met anyway, so shouldn't be a requirement anymore), and let auto disabling the whole iommu be the very very last resort. And also give a good warning/info about the insecurity and what exactly is wrong if possible, to report to vendors. -- Sander Monday, January 25, 2010, 11:08:15 AM, you wrote: > Noboru Iwamatsu wrote: >> Hi, >> >> > No, we cannot ignore it if iommu=force. The invisible device may be >> > disabled, not really non-existent. it is possibly that it is re-enabled >> > by malfunctional s/w. So when iommu=force, we should not ignore any >> > DRHD. We ignores it just to workaround the BIOS issue you encountered. >> >> OK, I return to the same question as Pasi asked. >> You mean even ignoring the DRHD that has no existent devices is >> insecure, right? >> In other word, iommu=1 might be insecure while working with workaround. >> > Yes, from the VT-d point of view, there might be a device not protected > by any DRHD if the device is re-enabled by malicious s/w and its DRHD is > ignored. >> We might have to consider security and BIOS workaround separately. >> I believe default action must be secure and enabled with strictly >> checked values. >> If "force" or some workaround options (e.g ignore_bogus_rmrr, >> ignore_bogus_drhd, force_enable_with_bogus_drhd, ...) >> specified, VT-d enabled with some special workaround, with >> uncertain values, but these mode should be considered >> "not always secure". >> > In order to still boot Xen, we will disable VT-d if detect some BIOS > bugs, such as incorrect RMRR. iommu=force was introduced to enable VT-d > anyway for security purpose, that means it won't allow to disable VT-d > to boot Xen. I agree to do the strict check for security by default, > Maybe we can add an option like "workaround_bogus_bios" which will try > to workaround known BIOS issues, such as ignore DRHD. I don't prefer to > add too many options like ignore_bogus_rmrr and ignore_bogus_drhd, it's > not practical for end users. > My suggestion is like: > iommu=1 (default): won't ignore any DRHD. when detect non-existent > devices under DRHD's scope, and find incorrect RMRR setting, disable > whole VT-d with warning message. This guarantees security when VT-d is > enabled, or just disable VT-d to let Xen work without VT-d because there > are users who don't need VT-d. > iommu=force: keep the same behavior. that's make sure VT-d enabled. It > won't ignore any DRHD, and if VT-d is disabled due to above BIOS issues, > it will quit Xen boot with warning message. > iommu=workaround_bogus_bios: if "all" devices under scope, ignore the > DRHD, if "some" devices under scope, disable whole VT-d in Xen. This > might be insecure because there might be a device not protected by any > DRHD if the device is re-enabled by malicious s/w. This is for user who > want to use VT-d regardless of security. > Welcome comments. > Regards, > Weidong >> What do you think? >> >> Regards, >> Noboru. >> >> >>> Noboru Iwamatsu wrote: >>> >>>> Weidong, >>>> >>>> I read the patch and the following thread. >>>> >>>> I understood what you mean, but I think it's better to >>>> limit the scope of "force_iommu". >>>> And I believe RMRR should be checked as same as DRHD. >>>> >>>> What I thought about DRHD is: >>>> If all devices under the scope of the DRHD are non-existent, >>>> this DRHD is invalid but safely ignorable, so ignore it. >>>> >>> No, we cannot ignore it if iommu=force. The invisible device may be >>> disabled, not really non-existent. it is possibly that it is re-enabled >>> by malfunctional s/w. So when iommu=force, we should not ignore any >>> DRHD. We ignores it just to workaround the BIOS issue you encountered. >>> >>>> If some devices under the scope of the DRHD are non-existent, >>>> this DRHD is invalid, so disable VT-d unless "iommu=force" >>>> option is specified. >>>> When "iommu=force" option is specified, even the invalid DRHD >>>> will be registered, because DRHD that has some existent devices >>>> must not be ignored due to security reasons. >>>> >>>> About the RMRR: >>>> If all devices under the scope of the RMRR are non-existent, >>>> this RMMR is invalid but ignorable, so ignore it. >>>> If some devices under the scope of the RMRR are non-existent, >>>> this RMRR is invalid, so disable VT-d unless "iommu=force" >>>> >>> RMRR is much different from DRHD, it's just reversed memories for >>> specific devices (now only Intel IGD and USB contollers need RMRR), it's >>> no security issue like described above. >>> if "all" devices under the scope of the RMRR are non-existent, we can >>> ignore the RMRR because no devices will use it. >>> if some" devices under the scope of the RMRR are non-existent, we cannot >>> ignore the RMRR, because there are still some devices want to use it. I >>> think we needn't to disable VT-d because it won't cause any issues. Of >>> course, we also can disable VT-d for this case strictly. >>> >>>> option is specified. When "iommu=force" option is specified, >>>> the invalid RMRR is ignored (it's safe). >>>> >>>> I attach the patch. >>>> >>>> What do you think? >>>> >>> Noboru, >>> >>> I think it need not to change current code. BTW, your patch is not based >>> on latest Xen. >>> >>> Regards, >>> Weidong >>> >>> >>> >>>> Regards, >>>> Noboru. >>>> >>>> >>>>> I implemented a patch and attached. >>>>> >>>>> patch description: >>>>> In order to make Xen more defensive to VT-d related BIOS issue, this >>>>> patch ignores a DRHD if all devices under its scope are not pci >>>>> discoverable, and regards a DRHD as invalid and then disable whole VT-d >>>>> if some devices under its scope are not pci discoverable. But if >>>>> iommu=force is set, it will enable all DRHDs reported by BIOS, to avoid >>>>> any security vulnerability with malicious s/s re-enabling "supposed >>>>> disabled" devices. Pls note that we don't know the devices under the >>>>> "Include_all" DRHD are existent or not, because the scope of >>>>> "Include_all" DRHD won't enumerate common pci device, it only enumerates >>>>> I/OxAPIC and HPET devices. >>>>> >>>>> Signed-off-by: Noboru Iwamatsu <n_iwamatsu@xxxxxxxxxxxxxx> >>>>> Signed-off-by: Weidong Han <weidong.han@xxxxxxxxx> >>>>> >>>>> >>>>> Noboru, pls test the patch on your machine? >>>>> >>>>> Joe, could you review the patch? and pls ACK it if it's fine for you. >>>>> >>>>> Regards, >>>>> Weidong >>>>> >>>>> Noboru Iwamatsu wrote: >>>>> >>>>>> Thanks, >>>>>> >>>>>> I understood. >>>>>> >>>>>> >>>>>>> Noboru Iwamatsu wrote: >>>>>>> >>>>>>>> Hi Weidong, >>>>>>>> >>>>>>>> I'm not sure why the security problem is caused by ignoring >>>>>>>> the DRHD that has only non-existent devices. >>>>>>>> >>>>>>>> Could you explain details or where to read the spec? >>>>>>>> >>>>>>> It's requested from security experts. The device that is not pci >>>>>>> discoverable may be re-enabled by malicious software. If its DRHD >>>>>>> is not >>>>>>> enabled, the re-enabled device is not protected by VT-d. It will cause >>>>>>> security issue. >>>>>>> >>>>>>> >>>>>>>> As you saying, security is the top-priority. >>>>>>>> However, when iommu=force is specified, we should enable vt-d >>>>>>>> if there are some potential issues. >>>>>>>> Because users want to "force" anyway. >>>>>>>> >>>>>>> iommu=force was introduced to enable VT-d anyway for security >>>>>>> purpose. I >>>>>>> plan to still enable those DRHDs that includes non-existed device when >>>>>>> iommu=force, otherwise ignore them. >>>>>>> >>>>>>> Regards, >>>>>>> Weidong >>>>>>> >>>>>>>> Regards, >>>>>>>> Noboru. >>>>>>>> >>>>>>>> >>>>>>>>> Keir Fraser wrote: >>>>>>>>> >>>>>>>>>> If we want to keep iommu=1 as default, then it is unacceptable to >>>>>>>>>> fail to >>>>>>>>>> boot on a fairly wide range of modern systems. We have to >>>>>>>>>> warn-and-disable, >>>>>>>>>> partially or completely, unless iommu=force is specified. Or we >>>>>>>>>> need to >>>>>>>>>> revert to iommu=0 as the default. >>>>>>>>>> >>>>>>>>>> What do you think, Weidong? >>>>>>>>>> >>>>>>>>> Yes. I agree to warn-and-disable for these BIOS issues, and consider >>>>>>>>> security more when iommu=force. Therefore I will implement a patch >>>>>>>>> based >>>>>>>>> on Nororu's patch. >>>>>>>>> >>>>>>>>> Regards, >>>>>>>>> Weidong >>>>>>>>> >>>>>>>>> >>>>>>>>>> -- Keir >>>>>>>>>> >>>>>>>>>> On 21/01/2010 14:17, "Sander Eikelenboom" <linux@xxxxxxxxxxxxxx> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> Hello Weidong, >>>>>>>>>>> >>>>>>>>>>> The problem is most vendor's just don't fix it and ignore the >>>>>>>>>>> problem >>>>>>>>>>> completely. >>>>>>>>>>> Most often hiding them selves behind: come back when it's a >>>>>>>>>>> problem >>>>>>>>>>> with >>>>>>>>>>> Microsoft Windows, that the only single thing we support (and no >>>>>>>>>>> other >>>>>>>>>>> software, so no vmware, no xen, no linux, perhaps even no >>>>>>>>>>> hypervisor) >>>>>>>>>>> Well I don't know if the virtual pc in windows 7 supports an iommu >>>>>>>>>>> now, but it >>>>>>>>>>> didn't in the past as far as i know, so any complain bounces off, >>>>>>>>>>> and >>>>>>>>>>> there it >>>>>>>>>>> all seems to end for them. >>>>>>>>>>> >>>>>>>>>>> Besides that i don't know if they do know what the problems with >>>>>>>>>>> there >>>>>>>>>>> implementation in BIOS is when someone reports it. >>>>>>>>>>> I think some behind the scenes pressure from Intel to vendors >>>>>>>>>>> might >>>>>>>>>>> help to >>>>>>>>>>> solve some of them. >>>>>>>>>>> (my Q35 chipset, "Intel V-PRO" marketed motherboard (so much for >>>>>>>>>>> that) also >>>>>>>>>>> suffers RMRR problem when another graphics card is inserted which >>>>>>>>>>> switches off >>>>>>>>>>> the IGD). >>>>>>>>>>> >>>>>>>>>>> Although i think in my case your patch will work around that >>>>>>>>>>> for me. >>>>>>>>>>> Perhaps a >>>>>>>>>>> third option is needed, which does all the workarounds possible >>>>>>>>>>> and >>>>>>>>>>> warns >>>>>>>>>>> about potential security problem when requested ? >>>>>>>>>>> >>>>>>>>>>> -- >>>>>>>>>>> Sander >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Thursday, January 21, 2010, 1:46:39 PM, you wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> Noboru Iwamatsu wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Hi Weidong, >>>>>>>>>>>>> >>>>>>>>>>>>> I re-send the DRHD-fix patch. >>>>>>>>>>>>> >>>>>>>>>>>>> If DRHD does not have existent devices, ignore it. >>>>>>>>>>>>> If DRHD has both existent and non-existent devices, consider it >>>>>>>>>>>>> invalid >>>>>>>>>>>>> and not register. >>>>>>>>>>>>> >>>>>>>>>>>> Although you patch workarounds your buggy BIOS, but we still >>>>>>>>>>>> need to >>>>>>>>>>>> enable it for security purpose as I mentioned in previous >>>>>>>>>>>> mail. We >>>>>>>>>>>> needn't workaround / fix all BIOS issues in software. I think >>>>>>>>>>>> security >>>>>>>>>>>> is more important for this specific BIOS issue. Did you report >>>>>>>>>>>> the >>>>>>>>>>>> BIOS >>>>>>>>>>>> issue to your OEM vendor? maybe it's better to get it fixed in >>>>>>>>>>>> BIOS. >>>>>>>>>>>> Regards, >>>>>>>>>>>> Weidong >>>>>>>>>>>> >>>>>>>>>>>>> According to this patch and yours, my machine successfully >>>>>>>>>>>>> booted >>>>>>>>>>>>> with vt-d enabled. >>>>>>>>>>>>> >>>>>>>>>>>>> Signed-off-by: Noboru Iwamatsu <n_iwamatsu@xxxxxxxxxxxxxx> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>>> Keir Fraser wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>>> On 21/01/2010 10:19, "Weidong Han" <weidong.han@xxxxxxxxx> >>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Sorry this is typo. >>>>>>>>>>>>>>>>> I mean: >>>>>>>>>>>>>>>>> So, I think RMRR that has no-existent device is "invalid" >>>>>>>>>>>>>>>>> and whole RMRR should be ignored. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> looks reasonable. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Keir, I Acks Noboru's rmrr patch. Or do you want us to merge >>>>>>>>>>>>>>>> them to one >>>>>>>>>>>>>>>> patch? >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Merge them up, re-send with both sign-off and acked-by all in >>>>>>>>>>>>>>> one >>>>>>>>>>>>>>> email. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>>> Keir >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> Sorry, I disagree with Noboru after thinking it again. If the >>>>>>>>>>>>>> RMRR >>>>>>>>>>>>>> has >>>>>>>>>>>>>> both no-existent device and also has existent devices in its >>>>>>>>>>>>>> scope, we >>>>>>>>>>>>>> should not ignore it because the existent devices under its >>>>>>>>>>>>>> scope >>>>>>>>>>>>>> will >>>>>>>>>>>>>> be impacted without the RMRR. so I suggest to print a warning >>>>>>>>>>>>>> instead of >>>>>>>>>>>>>> ignore it. Attached a patch for it. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Signed-off-by: Weidong Han <weidong.han@xxxxxxxxx> >>>>>>>>>>>>>> >> >> >> >> -- Best regards, Sander mailto:linux@xxxxxxxxxxxxxx _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.