On Thu, Sep 22, 2022 at 02:23:22PM -0300, Jason Gunthorpe wrote:
> > +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.
OK. Let me resend a version dropping this one. Thanks!
