| From b51b75f0604f17c0f6f3b6f68f1a521a5cc6b04f Mon Sep 17 00:00:00 2001 |
| From: Reinette Chatre <reinette.chatre@intel.com> |
| Date: Wed, 7 Dec 2022 14:52:20 -0800 |
| Subject: dmaengine: idxd: Let probe fail when workqueue cannot be enabled |
| |
| From: Reinette Chatre <reinette.chatre@intel.com> |
| |
| commit b51b75f0604f17c0f6f3b6f68f1a521a5cc6b04f upstream. |
| |
| The workqueue is enabled when the appropriate driver is loaded and |
| disabled when the driver is removed. When the driver is removed it |
| assumes that the workqueue was enabled successfully and proceeds to |
| free allocations made during workqueue enabling. |
| |
| Failure during workqueue enabling does not prevent the driver from |
| being loaded. This is because the error path within drv_enable_wq() |
| returns success unless a second failure is encountered |
| during the error path. By returning success it is possible to load |
| the driver even if the workqueue cannot be enabled and |
| allocations that do not exist are attempted to be freed during |
| driver remove. |
| |
| Some examples of problematic flows: |
| (a) |
| |
| idxd_dmaengine_drv_probe() -> drv_enable_wq() -> idxd_wq_request_irq(): |
| In above flow, if idxd_wq_request_irq() fails then |
| idxd_wq_unmap_portal() is called on error exit path, but |
| drv_enable_wq() returns 0 because idxd_wq_disable() succeeds. The |
| driver is thus loaded successfully. |
| |
| idxd_dmaengine_drv_remove()->drv_disable_wq()->idxd_wq_unmap_portal() |
| Above flow on driver unload triggers the WARN in devm_iounmap() because |
| the device resource has already been removed during error path of |
| drv_enable_wq(). |
| |
| (b) |
| |
| idxd_dmaengine_drv_probe() -> drv_enable_wq() -> idxd_wq_request_irq(): |
| In above flow, if idxd_wq_request_irq() fails then |
| idxd_wq_init_percpu_ref() is never called to initialize the percpu |
| counter, yet the driver loads successfully because drv_enable_wq() |
| returns 0. |
| |
| idxd_dmaengine_drv_remove()->__idxd_wq_quiesce()->percpu_ref_kill(): |
| Above flow on driver unload triggers a BUG when attempting to drop the |
| initial ref of the uninitialized percpu ref: |
| BUG: kernel NULL pointer dereference, address: 0000000000000010 |
| |
| Fix the drv_enable_wq() error path by returning the original error that |
| indicates failure of workqueue enabling. This ensures that the probe |
| fails when an error is encountered and the driver remove paths are only |
| attempted when the workqueue was enabled successfully. |
| |
| Fixes: 1f2bb40337f0 ("dmaengine: idxd: move wq_enable() to device.c") |
| Signed-off-by: Reinette Chatre <reinette.chatre@intel.com> |
| Reviewed-by: Dave Jiang <dave.jiang@intel.com> |
| Reviewed-by: Fenghua Yu <fenghua.yu@intel.com> |
| Cc: stable@vger.kernel.org |
| Link: https://lore.kernel.org/r/e8d8116e5efa0fd14fadc5adae6ffd319f0e5ff1.1670452419.git.reinette.chatre@intel.com |
| Signed-off-by: Vinod Koul <vkoul@kernel.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/dma/idxd/device.c | 3 +-- |
| 1 file changed, 1 insertion(+), 2 deletions(-) |
| |
| --- a/drivers/dma/idxd/device.c |
| +++ b/drivers/dma/idxd/device.c |
| @@ -1391,8 +1391,7 @@ err_res_alloc: |
| err_irq: |
| idxd_wq_unmap_portal(wq); |
| err_map_portal: |
| - rc = idxd_wq_disable(wq, false); |
| - if (rc < 0) |
| + if (idxd_wq_disable(wq, false)) |
| dev_dbg(dev, "wq %s disable failed\n", dev_name(wq_confdev(wq))); |
| err: |
| return rc; |