crypto: qat - Fix ADF_DEV_RESET_SYNC memory leak
authorHerbert Xu <herbert@gondor.apana.org.au>
Wed, 8 May 2024 08:39:51 +0000 (16:39 +0800)
committerHerbert Xu <herbert@gondor.apana.org.au>
Fri, 17 May 2024 10:55:07 +0000 (18:55 +0800)
Using completion_done to determine whether the caller has gone
away only works after a complete call.  Furthermore it's still
possible that the caller has not yet called wait_for_completion,
resulting in another potential UAF.

Fix this by making the caller use cancel_work_sync and then freeing
the memory safely.

Fixes: 7d42e097607c ("crypto: qat - resolve race condition during AER recovery")
Cc: <stable@vger.kernel.org> #6.8+
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Reviewed-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
drivers/crypto/intel/qat/qat_common/adf_aer.c

index 9da2278bd5b7dc594076478abf5387ed7e7ddbe0..04260f61d04294b24d9bf77b68789955de414f01 100644 (file)
@@ -130,8 +130,7 @@ static void adf_device_reset_worker(struct work_struct *work)
        if (adf_dev_restart(accel_dev)) {
                /* The device hanged and we can't restart it so stop here */
                dev_err(&GET_DEV(accel_dev), "Restart device failed\n");
-               if (reset_data->mode == ADF_DEV_RESET_ASYNC ||
-                   completion_done(&reset_data->compl))
+               if (reset_data->mode == ADF_DEV_RESET_ASYNC)
                        kfree(reset_data);
                WARN(1, "QAT: device restart failed. Device is unusable\n");
                return;
@@ -147,16 +146,8 @@ static void adf_device_reset_worker(struct work_struct *work)
        adf_dev_restarted_notify(accel_dev);
        clear_bit(ADF_STATUS_RESTARTING, &accel_dev->status);
 
-       /*
-        * The dev is back alive. Notify the caller if in sync mode
-        *
-        * If device restart will take a more time than expected,
-        * the schedule_reset() function can timeout and exit. This can be
-        * detected by calling the completion_done() function. In this case
-        * the reset_data structure needs to be freed here.
-        */
-       if (reset_data->mode == ADF_DEV_RESET_ASYNC ||
-           completion_done(&reset_data->compl))
+       /* The dev is back alive. Notify the caller if in sync mode */
+       if (reset_data->mode == ADF_DEV_RESET_ASYNC)
                kfree(reset_data);
        else
                complete(&reset_data->compl);
@@ -191,10 +182,10 @@ static int adf_dev_aer_schedule_reset(struct adf_accel_dev *accel_dev,
                if (!timeout) {
                        dev_err(&GET_DEV(accel_dev),
                                "Reset device timeout expired\n");
+                       cancel_work_sync(&reset_data->reset_work);
                        ret = -EFAULT;
-               } else {
-                       kfree(reset_data);
                }
+               kfree(reset_data);
                return ret;
        }
        return 0;