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

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



Hello Weidong,

Is it possible to enable/disable DRHD's and RMRR's after boot ?

For example if one would hotplug a pci device, that wasn't existent on boot ..
What would happen considering security ?
Is it possible to enable DRHD for that device although it was non existent at 
boot ?

--
Sander



Monday, January 25, 2010, 8:56:24 AM, you wrote:

> 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®.