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;
Date:   Thu, 22 Sep 2022 14:23:22 -0300
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     Nicolin Chen <nicolinc@nvidia.com>
Cc:     agross@kernel.org, bjorn.andersson@linaro.org,
        konrad.dybcio@somainline.org, joro@8bytes.org, will@kernel.org,
        robin.murphy@arm.com, sricharan@codeaurora.org,
        kevin.tian@intel.com, linux-arm-msm@vger.kernel.org,
        iommu@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 1/6] iommu/msm: Fix error-out routine in
 msm_iommu_attach_dev()
Message-ID: <YyyaCoHC95yyJpOK@nvidia.com>
References: <cover.1663836372.git.nicolinc@nvidia.com>
 <23e009bba72c3e46320c59acefbbdd976111cc8a.1663836372.git.nicolinc@nvidia.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <23e009bba72c3e46320c59acefbbdd976111cc8a.1663836372.git.nicolinc@nvidia.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?NguzxU8cBBsv9c2hTfVIXSCsRDaJ2gw1ZMq+yGd35ChUYSEJLjOPW74mXlo1?=
 =?us-ascii?Q?n/r6Zz+5WL1r1260aW8WyKCwIpTaWeUopSiPGBbKxRG5/aDhaB7+93DCX+Oy?=
 =?us-ascii?Q?uXqHQ/o7uHSxjqag6AehOWGLvYzX8RCmG+/7N3UV+nxzVm3DjpyHds0BhAbN?=
 =?us-ascii?Q?sDJLVPyQCNaNRbZOOm+CDopec1Z+2EpXt/i+bc9r8BPVK0wiyz4Vk5spa/w9?=
 =?us-ascii?Q?5QjVu+fFftSejvl6iz96BC+rRrR/kh4gy5m8Iig5bB9IS7J0k9qH0jozOn9b?=
 =?us-ascii?Q?PB/T+oxFexDb1UBJIsav6h7GKXXkDVDcrk2IIdqrfT3Ke1lrjfgwmklCDpUA?=
 =?us-ascii?Q?8wh1SgsXX427YCBs3/eHlhg2rNUhs4rrwH5Rtb8bQiN7Fe2k/1cti+tSfY1x?=
 =?us-ascii?Q?DjjjQN30QN1f+tzkO0/yF8Wy91L8b2LGiBh8GCGqlB7CgQzUdiY7sH9MkXqq?=
 =?us-ascii?Q?4FxNyZ8Y9GE9cJITXp5XD/dkQ1/P0G1OaPejYEg6jxZ+q0fLtLsW73u3v4Nv?=
 =?us-ascii?Q?E+0GKqMGIfHZOL/fAESIPB1IfqPj3L4VTbxPHXsRu3mMD2AzmNSdk834oZjR?=
 =?us-ascii?Q?R9fWUVc8+yDK1YsGYEKDAhjDWkQhXoUKjShMJ3EyK25Z4GuwVrIA+1hvaoYI?=
 =?us-ascii?Q?p66Awpy40CxFEkdg8QHGDraATAm0AQ5z9LqgFredVOO6dFHHIYjLdqRsyxsq?=
 =?us-ascii?Q?OHCw5hhYpJNX/kbKa6KvXrnvc0LDUNuECPZTLEh7mIQAc/f3xt+rMHV/basC?=
 =?us-ascii?Q?yBuoKGeVcnyxM3YJccuT+8Ku1q9EteeFFSjACEZXYLMc5wIqr68EjpUot64j?=
 =?us-ascii?Q?IHoUXyTsFZ5UZk0s4lb+S0eAh5XL0m2hTfC3agp+keX4L6rIMinXo/ZmVQ1r?=
 =?us-ascii?Q?ifxVNGnv2j/DQhp//7cGo5gcVK3oxI/fORad6hthKUky+KsSjVPydSz+MzZx?=
 =?us-ascii?Q?Yxd9n8nbksBXg33gcSWfh/AozxmJAqz/T1IH/wTtOJnU8hdil5k/o0Guc5us?=
 =?us-ascii?Q?tkmbHqx+X6zBOn3Up1NmWh3H/dKc0PBib4dOQUlhoc2e7NozBi4sTMQns3K1?=
 =?us-ascii?Q?TyW85U2Ezx9P89WKylPluUxXE8oQ9765LZVl126qkfOXsk8tP9lfB4RCdzM/?=
 =?us-ascii?Q?cPQYT9Yb0z4i/XKbGq8VI17Wtgja+W24KJ97QlJ7PVdvxWxDJwTdCZ4I0dNA?=
 =?us-ascii?Q?ugj0R1kaa3XHHJ2eK//OD70k7Ln7dlS5DGAEcow2+I7jbhU3nr5AuPDrcLvo?=
 =?us-ascii?Q?xg33t1jEVNeWeagi0MnCcA2LU7rrkGOReA1mfQ76AXsRyP3qYFK+37RlYy5T?=
 =?us-ascii?Q?jrb5wzJvn2OR5UUyjd/+bTlLccFigAFslT8tB99tVXJWP7oNVlKw9SqTsqAm?=
 =?us-ascii?Q?yJHo8RkQda0FgEr1GMHVGXvfVFtgyiUa297/HdnVxjKTZPCQSonII4+n8KZz?=
 =?us-ascii?Q?fwu3PhoRtCVseO/HjwWqykox/LHpUfZy5cb3RYIwW/ZPMTgZbdrmZwkOsAnT?=
 =?us-ascii?Q?pQbcSK9rqy+fT0svlkaaW9cNH2ChtLx4cGR03MX7DP61T5pm/FApvGMdkjWv?=
 =?us-ascii?Q?J2sY7rS7CrTpd9ksMGi/gvN04d+kqciZOayqTKoj?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2f8080aa-2622-4a46-0230-08da9cbf2732
X-MS-Exchange-CrossTenant-AuthSource: MN2PR12MB4192.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Sep 2022 17:23:23.8184
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: zYJvcZqLiTt2BmlfxOzg1IMmXDGaQMcTWEcydga7EeYn42Tqws+F8NYTEqh8ksMJ
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR12MB7596
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE
        autolearn=no 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

On Thu, Sep 22, 2022 at 01:53:44AM -0700, Nicolin Chen wrote:

> diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
> index 6a24aa804ea3..14df722f0060 100644
> --- a/drivers/iommu/msm_iommu.c
> +++ b/drivers/iommu/msm_iommu.c
> @@ -394,6 +394,34 @@ static struct iommu_device *msm_iommu_probe_device(struct device *dev)
>  	return &iommu->iommu;
>  }
>  
> +static void msm_iommu_detach_dev(struct iommu_domain *domain,
> +				 struct device *dev)
> +{
> +	struct msm_priv *priv = to_msm_priv(domain);
> +	unsigned long flags;
> +	struct msm_iommu_dev *iommu;
> +	struct msm_iommu_ctx_dev *master;
> +	int ret;
> +
> +	free_io_pgtable_ops(priv->iop);
> +
> +	spin_lock_irqsave(&msm_iommu_lock, flags);
> +	list_for_each_entry(iommu, &priv->list_attached, dom_node) {
> +		ret = __enable_clocks(iommu);
> +		if (ret)
> +			goto fail;
> +
> +		list_for_each_entry(master, &iommu->ctx_list, list) {
> +			msm_iommu_free_ctx(iommu->context_map, master->num);
> +			__reset_context(iommu->base, master->num);

This isn't safe if the loop in msm_iommu_attach_dev() exits
early. 

Firstly the it leaves master->num set to -ERRNO so clear_bit will
corrupt memory

Secondly if the attach loop gets half way through the remaining
entries have master->num == 0 and so the clear_bit() will corrupt an
otherwise valid entry.

Fixing all of this properly looks like a big deal, I think we should
drop this patch from the series.

Jason