[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.