Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Tian, Kevin" <kevin.tian@intel.com>
To:     Lu Baolu <baolu.lu@linux.intel.com>,
        Joerg Roedel <joro@8bytes.org>,
        "Jason Gunthorpe" <jgg@nvidia.com>,
        Christoph Hellwig <hch@infradead.org>,
        "Will Deacon" <will@kernel.org>,
        Robin Murphy <robin.murphy@arm.com>,
        "Jean-Philippe Brucker" <jean-philippe@linaro.org>
CC:     Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
        Hector Martin <marcan@marcan.st>,
        Sven Peter <sven@svenpeter.dev>,
        Rob Clark <robdclark@gmail.com>,
        Marek Szyprowski <m.szyprowski@samsung.com>,
        "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Yong Wu <yong.wu@mediatek.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Heiko Stuebner <heiko@sntech.de>,
        Matthew Rosato <mjrosato@linux.ibm.com>,
        Orson Zhai <orsonzhai@gmail.com>,
        Baolin Wang <baolin.wang@linux.alibaba.com>,
        "Chunyan Zhang" <zhang.lyra@gmail.com>,
        Chen-Yu Tsai <wens@csie.org>,
        Thierry Reding <thierry.reding@gmail.com>,
        "iommu@lists.linux.dev" <iommu@lists.linux.dev>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v3 08/20] iommu/sprd: Remove detach_dev callback
Thread-Topic: [PATCH v3 08/20] iommu/sprd: Remove detach_dev callback
Thread-Index: AQHZAvZZiCOMgUZ45U6s9kgD0YOasK5VPdYw
Date:   Tue, 29 Nov 2022 03:34:57 +0000
Message-ID: <BL1PR11MB5271FF59EEAC72D3C4A04E538C129@BL1PR11MB5271.namprd11.prod.outlook.com>
References: <20221128064648.1934720-1-baolu.lu@linux.intel.com>
 <20221128064648.1934720-9-baolu.lu@linux.intel.com>
In-Reply-To: <20221128064648.1934720-9-baolu.lu@linux.intel.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?QGQwyg8XgI2nnfRL3KRVisT2Z1rjmKq8PIhoiIz/vwZq1NvbTS/qI7Y7mAUe?=
 =?us-ascii?Q?4gNC4HjD1jIaprp3xGaD9jZmRLeglo1NM7BQzOjqbw/tK8xEGzEhe4Br925N?=
 =?us-ascii?Q?nHL128YCMksjzzw6aNfq5MK8E2hsgIDRJZEsLZm4EN4k3tYXVeXXM7exZLTa?=
 =?us-ascii?Q?aVvGsjM1yjKdw7e9PmzxzddzIBjY5kEL9xlrrrT51XX9csok4qEIkV+u4FrM?=
 =?us-ascii?Q?EU5bAfY1aGsjrAhoGtS9FWvLKM3jPQ5sj7elsvEkRBXA/pc8Gr6szFF301zI?=
 =?us-ascii?Q?OyqevV70vS1hgIKZjAbYCFyttH9RLWlDTWwGClo+l1EVFzjOvAMqL3b33Cyl?=
 =?us-ascii?Q?0xE5oOosSJkWvzpgX4VEk9w6DKrbR7QqwQX4/Ea0veWDYY310XB3QkXKcUo6?=
 =?us-ascii?Q?mjIUbTnCjNaCes0VkfZ9kCwl7TyTLbiEwAauHPhTy/WOwCaLyLdWY1t+UbTZ?=
 =?us-ascii?Q?1W3EaZ0xzwUb3jCW1qI2XzULsEvJx1EAyRQKaUft9V6lRLjJrUa0THtVPORr?=
 =?us-ascii?Q?htFhHcTm0fkoS7ZiFtr0RPLDp6GSn2is4W/lDQ2uc1wjBwDBddzoEksCHCSK?=
 =?us-ascii?Q?nSzYQZz75QVd6UPB4R6oIWp5NMU6yn3ZzjXxaiK+UVdf7k/iFTy1lCxNDcWp?=
 =?us-ascii?Q?I/HAPYDWGL/IZFBjhwwYPgiGUJPO4gxOcstu+HEwMwfdSyhJYMDe4O3SsHdr?=
 =?us-ascii?Q?CJ3jGcvBYDl+VriNX/9bI5OyLoJ1g4/H55D7YlNKkXl2cYXE4R1ODX7VBh0i?=
 =?us-ascii?Q?rPX56If0vBz0bmJK/QU6lbMeQT3OzDrBL+m6XVhMsQDM8fDNRKTPrYwdGJ0p?=
 =?us-ascii?Q?PWgo33OTwNlyNftV6zjQPoR9im36SlI6u7CREypIixT5V4yGZAP1GwZh3xbD?=
 =?us-ascii?Q?ZHN1wCbMPBH5O+Ni7cBFCnpjND85nG3s6qNMgUReWVgrWx/29DfH63irOAtK?=
 =?us-ascii?Q?ctOd1TcS4sRGIqcFoveFVgXfLpaKAOuEMhkLIoH0e1N9dtuGCCkbDgmc95H1?=
 =?us-ascii?Q?CEXhD+UnGGazAU6h6E8N9ax7tUHvilZ+t14ySkkwSVaC/Cw3dDTpVqi6kq9T?=
 =?us-ascii?Q?ULo6Vtd+FSX7qS3aKjp/quZgcLNxhT7LRpLxEO+3qxjllUDDMC1e7j/m2zy9?=
 =?us-ascii?Q?KXxIXkUqkXt2sHQQSXxpGd57UebwJoRs3Aj750mz7/VBcjJCfrQmu9YWDk26?=
 =?us-ascii?Q?0gTV7j00TVxy+DTD4ASuEqpy9xr/53HBebm7tm13Zz67T/90GajCNka4e4+8?=
 =?us-ascii?Q?CShBYVHAGSGA/CMIclYBa+luyjuZa8rrGzIssTtXvqPZI8aNdQdzqSMAEH4P?=
 =?us-ascii?Q?ztaelJ84C74c++1CQVYGM83g5YujInC6EvGKRpVUhlXbyNrFx34f6xOAPXOr?=
 =?us-ascii?Q?hCPRqZn9ICQZwy3bTwhGV8ZGBanG2cVGLaJfmWGkvmPmGQ0Q+w7d3ZZ8gZPm?=
 =?us-ascii?Q?TYxYcNLi2lUkAhuIm2EAuLw+hXdmCWigU11pdnhZWpTs3i8kjmx49BmO1+ib?=
 =?us-ascii?Q?6EgnfH6Tg3e81qVVNz+Zn5itK++JGfL99RIyxslsHU+lOTmXRtN9wUE8GSGs?=
 =?us-ascii?Q?fIbdBZ5S0djUQKrcP2MYEWyutnqTHul8LxTVIVlc?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5271.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 424dad79-1bb0-418c-1bc0-08dad1bab020
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Nov 2022 03:34:57.4894
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: EFPR7GK7NzhEqm59L1CZUSFlPY9sctBhTigBTgkio/4x2ld2AL19WiMAnDJf+CgeUf85c6MxqGYg8E5rbCSutQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB4756
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Monday, November 28, 2022 2:47 PM
>=20
> The IOMMU driver supports default domain, so the detach_dev op will never
> be called. Remove it to avoid dead code.
>=20
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/iommu/sprd-iommu.c | 16 ----------------
>  1 file changed, 16 deletions(-)
>=20
> 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;
>  }
>=20
> -static void sprd_iommu_detach_device(struct iommu_domain *domain,
> -					     struct device *dev)
> -{
> -	struct sprd_iommu_domain *dom =3D to_sprd_domain(domain);
> -	struct sprd_iommu_device *sdev =3D dom->sdev;
> -	size_t pgt_size =3D 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 =3D 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.