linux-kernel - Re: [PATCH] iommu/vt-d: Do not BUG_ON in intel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140804114205.GA5545@pd.tnic>
Date:	Mon, 4 Aug 2014 13:42:05 +0200
From:	Borislav Petkov <bp@...en8.de>
To:	Joerg Roedel <joro@...tes.org>
Cc:	iommu@...ts.linux-foundation.org, linux-kernel@...r.kernel.org,
	Joerg Roedel <jroedel@...e.de>,
	Jiang Liu <jiang.liu@...ux.intel.com>,
	David Woodhouse <dwmw2@...radead.org>
Subject: Re: [PATCH] iommu/vt-d: Do not BUG_ON in intel_unmap if no domain

On Mon, Aug 04, 2014 at 01:23:06PM +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@...e.de>
> 
> This BUG_ON is easy to trigger with device-hotplug (e.g.
> SR-IOV). The device_notifier function in the Intel IOMMU
> driver listens to the BUS_NOTIFY_DEL_DEVICE event and frees
> the domain for the device if it is reveived.
> 
> But this event is triggered before the device driver is
> unbound from the device. When the driver core actually
> removes the device the driver may release pending DMA
> resources, which ends up in intel_unmap and triggers the
> BUG_ON.
> 
> Not listening to BUS_NOTIFY_DEL_DEVICE would cause resource
> leakage with devices that have never been assigned to any
> driver, so fix this issue by just making unmap a nop when
> the domain is already released.
> 
> Cc: Jiang Liu <jiang.liu@...ux.intel.com>
> Cc: David Woodhouse <dwmw2@...radead.org>
> Signed-off-by: Joerg Roedel <jroedel@...e.de>
> ---
>  drivers/iommu/intel-iommu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index d1f5caa..7d689d7 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -3196,7 +3196,8 @@ static void intel_unmap(struct device *dev, dma_addr_t dev_addr)
>  		return;
>  
>  	domain = find_domain(dev);
> -	BUG_ON(!domain);
> +	if (!domain)
> +		return;

It is always questionable when people remove BUG_ONs because relaxing
assertions sound like a temporary fix more often than not. Sounds to me
that the original commit which deals with BUS_NOTIFY_DEL_DEVICE needs to
try again with the fix. :-)

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/