iommu/amd: Prevent possible null pointer dereference and infinite loop

The check for !dev_data->domain in __detach_device() emits a warning and
returns. The calling code in detach_device() dereferences dev_data->domain
afterwards unconditionally, so in case that dev_data->domain is NULL the
warning will be immediately followed by a NULL pointer dereference.

The calling code in cleanup_domain() loops infinite when !dev_data->domain
and the check in __detach_device() returns immediately because dev_list is
not changed.

do_detach() duplicates this check without throwing a warning.

Move the check with the explanation of the do_detach() code into the caller
detach_device() and return immediately. Throw an error, when hitting the
condition in cleanup_domain().

Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
Signed-off-by: Joerg Roedel <jroedel@suse.de>
This commit is contained in:
Anna-Maria Gleixner 2018-05-07 14:53:27 +02:00 коммит произвёл Joerg Roedel
Родитель 29a0c41541
Коммит ea3fd04028
1 изменённых файлов: 10 добавлений и 12 удалений

Просмотреть файл

@ -1915,15 +1915,6 @@ static void do_detach(struct iommu_dev_data *dev_data)
struct amd_iommu *iommu; struct amd_iommu *iommu;
u16 alias; u16 alias;
/*
* First check if the device is still attached. It might already
* be detached from its domain because the generic
* iommu_detach_group code detached it and we try again here in
* our alias handling.
*/
if (!dev_data->domain)
return;
iommu = amd_iommu_rlookup_table[dev_data->devid]; iommu = amd_iommu_rlookup_table[dev_data->devid];
alias = dev_data->alias; alias = dev_data->alias;
@ -2128,9 +2119,6 @@ static void __detach_device(struct iommu_dev_data *dev_data)
*/ */
WARN_ON(!irqs_disabled()); WARN_ON(!irqs_disabled());
if (WARN_ON(!dev_data->domain))
return;
domain = dev_data->domain; domain = dev_data->domain;
spin_lock(&domain->lock); spin_lock(&domain->lock);
@ -2152,6 +2140,15 @@ static void detach_device(struct device *dev)
dev_data = get_dev_data(dev); dev_data = get_dev_data(dev);
domain = dev_data->domain; domain = dev_data->domain;
/*
* First check if the device is still attached. It might already
* be detached from its domain because the generic
* iommu_detach_group code detached it and we try again here in
* our alias handling.
*/
if (WARN_ON(!dev_data->domain))
return;
/* lock device table */ /* lock device table */
spin_lock_irqsave(&amd_iommu_devtable_lock, flags); spin_lock_irqsave(&amd_iommu_devtable_lock, flags);
__detach_device(dev_data); __detach_device(dev_data);
@ -2797,6 +2794,7 @@ static void cleanup_domain(struct protection_domain *domain)
while (!list_empty(&domain->dev_list)) { while (!list_empty(&domain->dev_list)) {
entry = list_first_entry(&domain->dev_list, entry = list_first_entry(&domain->dev_list,
struct iommu_dev_data, list); struct iommu_dev_data, list);
BUG_ON(!entry->domain);
__detach_device(entry); __detach_device(entry);
} }