linux-kernel - RE: [PATCH v3 08/20] iommu/sprd: Remove detach

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: <BL1PR11MB5271FF59EEAC72D3C4A04E538C129@BL1PR11MB5271.namprd11.prod.outlook.com>
Date:   Tue, 29 Nov 2022 03:34:57 +0000
From:   "Tian, Kevin" <kevin.tian@...el.com>
To:     Lu Baolu <baolu.lu@...ux.intel.com>,
        Joerg Roedel <joro@...tes.org>,
        "Jason Gunthorpe" <jgg@...dia.com>,
        Christoph Hellwig <hch@...radead.org>,
        "Will Deacon" <will@...nel.org>,
        Robin Murphy <robin.murphy@....com>,
        "Jean-Philippe Brucker" <jean-philippe@...aro.org>
CC:     Suravee Suthikulpanit <suravee.suthikulpanit@....com>,
        Hector Martin <marcan@...can.st>,
        Sven Peter <sven@...npeter.dev>,
        Rob Clark <robdclark@...il.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        "Krzysztof Kozlowski" <krzysztof.kozlowski@...aro.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Yong Wu <yong.wu@...iatek.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Heiko Stuebner <heiko@...ech.de>,
        Matthew Rosato <mjrosato@...ux.ibm.com>,
        Orson Zhai <orsonzhai@...il.com>,
        Baolin Wang <baolin.wang@...ux.alibaba.com>,
        "Chunyan Zhang" <zhang.lyra@...il.com>,
        Chen-Yu Tsai <wens@...e.org>,
        Thierry Reding <thierry.reding@...il.com>,
        "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v3 08/20] iommu/sprd: Remove detach_dev callback

> From: Lu Baolu <baolu.lu@...ux.intel.com>
> Sent: Monday, November 28, 2022 2:47 PM
> 
> The IOMMU driver supports default domain, so the detach_dev op will never
> be called. Remove it to avoid dead code.
> 
> Signed-off-by: Lu Baolu <baolu.lu@...ux.intel.com>
> ---
>  drivers/iommu/sprd-iommu.c | 16 ----------------
>  1 file changed, 16 deletions(-)
> 
> diff --git a/drivers/iommu/sprd-iommu.c b/drivers/iommu/sprd-iommu.c
> index 219bfa11f7f4..ae94d74b73f4 100644
> --- a/drivers/iommu/sprd-iommu.c
> +++ b/drivers/iommu/sprd-iommu.c
> @@ -255,21 +255,6 @@ static int sprd_iommu_attach_device(struct
> iommu_domain *domain,
>  	return 0;
>  }
> 
> -static void sprd_iommu_detach_device(struct iommu_domain *domain,
> -					     struct device *dev)
> -{
> -	struct sprd_iommu_domain *dom = to_sprd_domain(domain);
> -	struct sprd_iommu_device *sdev = dom->sdev;
> -	size_t pgt_size = sprd_iommu_pgt_size(domain);
> -
> -	if (!sdev)
> -		return;
> -
> -	dma_free_coherent(sdev->dev, pgt_size, dom->pgt_va, dom-
> >pgt_pa);
> -	sprd_iommu_hw_en(sdev, false);
> -	dom->sdev = NULL;
> -}
> -

Looks this reveals a bug in this driver (not caused by this removal).

sprd_iommu_attach_device() doesn't check whether the device has
been already attached to a domain and do auto detach.

It's written in a way that .detach_dev() must be called to free the
dma buffer but ignores the fact that it's not called when default
domain support is claimed.

Then the dma buffer allocated for the previous domain was left
unhandled, causing memory leak.