crypto: qat - protect service table iterations with service_lock

The service_table list is protected by service_lock when entries are
added or removed (in adf_service_add() and adf_service_remove()), but
several functions iterate over the list without holding this lock.

A concurrent adf_service_register() or adf_service_unregister() call
could modify the list during traversal, leading to list corruption or
a use-after-free.

Fix this by holding service_lock across all list_for_each_entry()
iterations of service_table in adf_dev_init(), adf_dev_start(),
adf_dev_stop(), adf_dev_shutdown(), adf_dev_restarting_notify(),
adf_dev_restarted_notify(), and adf_error_notifier().

The lock ordering is safe: callers of the static helpers (adf_dev_up()
and adf_dev_down()) acquire state_lock before service_lock, and no
event_hld callback or service_lock holder ever acquires state_lock in
the reverse order.

Cc: stable@vger.kernel.org
Fixes: d8cba25d2c68 ("crypto: qat - Intel(R) QAT driver framework")
Signed-off-by: Ahsan Atta <ahsan.atta@intel.com>
Co-developed-by: Maksim Lukoshkov <maksim.lukoshkov@intel.com>
Signed-off-by: Maksim Lukoshkov <maksim.lukoshkov@intel.com>
Reviewed-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/drivers/crypto/intel/qat/qat_common/adf_init.c b/drivers/crypto/intel/qat/qat_common/adf_init.c
index f9f5696ed476bc6c9d74a8ed6daf9431025fca18..1c7f9e49914d438d53eaccf53a99c6643ac79e56 100644 (file)
--- a/drivers/crypto/intel/qat/qat_common/adf_init.c
+++ b/drivers/crypto/intel/qat/qat_common/adf_init.c
@@ -155,15 +155,18 @@ static int adf_dev_init(struct adf_accel_dev *accel_dev)
         * This is to facilitate any ordering dependencies between services
         * prior to starting any of the accelerators.
         */
+       mutex_lock(&service_lock);
        list_for_each_entry(service, &service_table, list) {
                if (service->event_hld(accel_dev, ADF_EVENT_INIT)) {
                        dev_err(&GET_DEV(accel_dev),
                                "Failed to initialise service %s\n",
                                service->name);
+                       mutex_unlock(&service_lock);
                        return -EFAULT;
                }
                set_bit(accel_dev->accel_id, service->init_status);
        }
+       mutex_unlock(&service_lock);
 
        return 0;
 }
@@ -233,15 +236,18 @@ static int adf_dev_start(struct adf_accel_dev *accel_dev)
        if (ret && ret != -EOPNOTSUPP)
                return ret;
 
+       mutex_lock(&service_lock);
        list_for_each_entry(service, &service_table, list) {
                if (service->event_hld(accel_dev, ADF_EVENT_START)) {
                        dev_err(&GET_DEV(accel_dev),
                                "Failed to start service %s\n",
                                service->name);
+                       mutex_unlock(&service_lock);
                        return -EFAULT;
                }
                set_bit(accel_dev->accel_id, service->start_status);
        }
+       mutex_unlock(&service_lock);
 
        clear_bit(ADF_STATUS_STARTING, &accel_dev->status);
        set_bit(ADF_STATUS_STARTED, &accel_dev->status);
@@ -315,6 +321,7 @@ static void adf_dev_stop(struct adf_accel_dev *accel_dev)
                qat_comp_algs_unregister(hw_data->accel_capabilities_ext_mask);
        clear_bit(ADF_STATUS_COMP_ALGS_REGISTERED, &accel_dev->status);
 
+       mutex_lock(&service_lock);
        list_for_each_entry(service, &service_table, list) {
                if (!test_bit(accel_dev->accel_id, service->start_status))
                        continue;
@@ -326,6 +333,7 @@ static void adf_dev_stop(struct adf_accel_dev *accel_dev)
                        clear_bit(accel_dev->accel_id, service->start_status);
                }
        }
+       mutex_unlock(&service_lock);
 
        if (hw_data->stop_timer)
                hw_data->stop_timer(accel_dev);
@@ -375,6 +383,7 @@ static void adf_dev_shutdown(struct adf_accel_dev *accel_dev)
                                  &accel_dev->status);
        }
 
+       mutex_lock(&service_lock);
        list_for_each_entry(service, &service_table, list) {
                if (!test_bit(accel_dev->accel_id, service->init_status))
                        continue;
@@ -385,6 +394,7 @@ static void adf_dev_shutdown(struct adf_accel_dev *accel_dev)
                else
                        clear_bit(accel_dev->accel_id, service->init_status);
        }
+       mutex_unlock(&service_lock);
 
        adf_rl_exit(accel_dev);
 
@@ -419,12 +429,14 @@ int adf_dev_restarting_notify(struct adf_accel_dev *accel_dev)
 {
        struct service_hndl *service;
 
+       mutex_lock(&service_lock);
        list_for_each_entry(service, &service_table, list) {
                if (service->event_hld(accel_dev, ADF_EVENT_RESTARTING))
                        dev_err(&GET_DEV(accel_dev),
                                "Failed to restart service %s.\n",
                                service->name);
        }
+       mutex_unlock(&service_lock);
        return 0;
 }
 
@@ -432,12 +444,14 @@ int adf_dev_restarted_notify(struct adf_accel_dev *accel_dev)
 {
        struct service_hndl *service;
 
+       mutex_lock(&service_lock);
        list_for_each_entry(service, &service_table, list) {
                if (service->event_hld(accel_dev, ADF_EVENT_RESTARTED))
                        dev_err(&GET_DEV(accel_dev),
                                "Failed to restart service %s.\n",
                                service->name);
        }
+       mutex_unlock(&service_lock);
        return 0;
 }
 
@@ -445,12 +459,14 @@ void adf_error_notifier(struct adf_accel_dev *accel_dev)
 {
        struct service_hndl *service;
 
+       mutex_lock(&service_lock);
        list_for_each_entry(service, &service_table, list) {
                if (service->event_hld(accel_dev, ADF_EVENT_FATAL_ERROR))
                        dev_err(&GET_DEV(accel_dev),
                                "Failed to send error event to %s.\n",
                                service->name);
        }
+       mutex_unlock(&service_lock);
 }
 
 int adf_dev_down(struct adf_accel_dev *accel_dev)