| Message ID | 20230509060716.2830630-1-harshit.m.mogalapalli@oracle.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0642287e3ecdd0d1f88e6a2e63768e16153a990c |
| Headers | show |
| Series | dmaengine: idxd: Fix passing freed memory in idxd_cdev_open() | expand |
On 5/8/23 11:07 PM, Harshit Mogalapalli wrote: > Smatch warns: > drivers/dma/idxd/cdev.c:327: > idxd_cdev_open() warn: 'sva' was already freed. > > When idxd_wq_set_pasid() fails, the current code unbinds sva and then > goes to 'failed_set_pasid' where iommu_sva_unbind_device is called > again causing the above warning. > [ device_user_pasid_enabled(idxd) is still true when calling > failed_set_pasid ] > > Fix this by removing additional unbind when idxd_wq_set_pasid() fails > > Fixes: b022f59725f0 ("dmaengine: idxd: add idxd_copy_cr() to copy user completion record during page fault handling") > Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> Acked-by: Dave Jiang <dave.jiang@intel.com> Thank you! > --- > This is purely based on static analysis. Only compile tested. > --- > drivers/dma/idxd/cdev.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c > index ecbf67c2ad2b..d32deb9b4e3d 100644 > --- a/drivers/dma/idxd/cdev.c > +++ b/drivers/dma/idxd/cdev.c > @@ -277,7 +277,6 @@ static int idxd_cdev_open(struct inode *inode, struct file *filp) > if (wq_dedicated(wq)) { > rc = idxd_wq_set_pasid(wq, pasid); > if (rc < 0) { > - iommu_sva_unbind_device(sva); > dev_err(dev, "wq set pasid failed: %d\n", rc); > goto failed_set_pasid; > }
On 5/8/23 23:07, Harshit Mogalapalli wrote: > Smatch warns: > drivers/dma/idxd/cdev.c:327: > idxd_cdev_open() warn: 'sva' was already freed. > > When idxd_wq_set_pasid() fails, the current code unbinds sva and then > goes to 'failed_set_pasid' where iommu_sva_unbind_device is called > again causing the above warning. > [ device_user_pasid_enabled(idxd) is still true when calling > failed_set_pasid ] > > Fix this by removing additional unbind when idxd_wq_set_pasid() fails > > Fixes: b022f59725f0 ("dmaengine: idxd: add idxd_copy_cr() to copy user completion record during page fault handling") > Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> Acked-by: Fenghua Yu <fenghua.yu@intel.com> Thanks. -Fenghua
On 08-05-23, 23:07, Harshit Mogalapalli wrote: > Smatch warns: > drivers/dma/idxd/cdev.c:327: > idxd_cdev_open() warn: 'sva' was already freed. > > When idxd_wq_set_pasid() fails, the current code unbinds sva and then > goes to 'failed_set_pasid' where iommu_sva_unbind_device is called > again causing the above warning. > [ device_user_pasid_enabled(idxd) is still true when calling > failed_set_pasid ] Applied, thanks
diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c index ecbf67c2ad2b..d32deb9b4e3d 100644 --- a/drivers/dma/idxd/cdev.c +++ b/drivers/dma/idxd/cdev.c @@ -277,7 +277,6 @@ static int idxd_cdev_open(struct inode *inode, struct file *filp) if (wq_dedicated(wq)) { rc = idxd_wq_set_pasid(wq, pasid); if (rc < 0) { - iommu_sva_unbind_device(sva); dev_err(dev, "wq set pasid failed: %d\n", rc); goto failed_set_pasid; }
Smatch warns: drivers/dma/idxd/cdev.c:327: idxd_cdev_open() warn: 'sva' was already freed. When idxd_wq_set_pasid() fails, the current code unbinds sva and then goes to 'failed_set_pasid' where iommu_sva_unbind_device is called again causing the above warning. [ device_user_pasid_enabled(idxd) is still true when calling failed_set_pasid ] Fix this by removing additional unbind when idxd_wq_set_pasid() fails Fixes: b022f59725f0 ("dmaengine: idxd: add idxd_copy_cr() to copy user completion record during page fault handling") Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> --- This is purely based on static analysis. Only compile tested. --- drivers/dma/idxd/cdev.c | 1 - 1 file changed, 1 deletion(-)