Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

crypto: qat - fix concurrency issue when device state changes

The sysfs `state` attribute is not protected against race conditions.
If multiple processes perform a device state transition on the same
device in parallel, unexpected behaviors might occur.

For transitioning the device state, adf_sysfs.c calls the functions
adf_dev_init(), adf_dev_start(), adf_dev_stop() and adf_dev_shutdown()
which are unprotected and interdependent on each other. To perform a
state transition, these functions needs to be called in a specific
order:
* device up: adf_dev_init() -> adf_dev_start()
* device down: adf_dev_stop() -> adf_dev_shutdown()

This change introduces the functions adf_dev_up() and adf_dev_down()
which wrap the state machine functions and protect them with a
per-device lock. These are then used in adf_sysfs.c instead of the
individual state transition functions.

Fixes: 5ee52118ac14 ("crypto: qat - expose device state through sysfs for 4xxx")
Signed-off-by: Shashank Gupta <shashank.gupta@intel.com>
Reviewed-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

authored by

Shashank Gupta and committed by
Herbert Xu
1bdc8555 59a0ab49

+73 -20
+1
drivers/crypto/qat/qat_common/adf_accel_devices.h
··· 310 310 u8 pf_compat_ver; 311 311 } vf; 312 312 }; 313 + struct mutex state_lock; /* protect state of the device */ 313 314 bool is_vf; 314 315 u32 accel_id; 315 316 };
+3
drivers/crypto/qat/qat_common/adf_common_drv.h
··· 58 58 void adf_dev_shutdown(struct adf_accel_dev *accel_dev); 59 59 int adf_dev_shutdown_cache_cfg(struct adf_accel_dev *accel_dev); 60 60 61 + int adf_dev_up(struct adf_accel_dev *accel_dev, bool init_config); 62 + int adf_dev_down(struct adf_accel_dev *accel_dev, bool cache_config); 63 + 61 64 void adf_devmgr_update_class_index(struct adf_hw_device_data *hw_data); 62 65 void adf_clean_vf_map(bool); 63 66
+2
drivers/crypto/qat/qat_common/adf_dev_mgr.c
··· 223 223 map->attached = true; 224 224 list_add_tail(&map->list, &vfs_table); 225 225 } 226 + mutex_init(&accel_dev->state_lock); 226 227 unlock: 227 228 mutex_unlock(&table_lock); 228 229 return ret; ··· 270 269 } 271 270 } 272 271 unlock: 272 + mutex_destroy(&accel_dev->state_lock); 273 273 list_del(&accel_dev->list); 274 274 mutex_unlock(&table_lock); 275 275 }
+64
drivers/crypto/qat/qat_common/adf_init.c
··· 400 400 401 401 return 0; 402 402 } 403 + 404 + int adf_dev_down(struct adf_accel_dev *accel_dev, bool reconfig) 405 + { 406 + int ret = 0; 407 + 408 + if (!accel_dev) 409 + return -EINVAL; 410 + 411 + mutex_lock(&accel_dev->state_lock); 412 + 413 + if (!adf_dev_started(accel_dev)) { 414 + dev_info(&GET_DEV(accel_dev), "Device qat_dev%d already down\n", 415 + accel_dev->accel_id); 416 + ret = -EINVAL; 417 + goto out; 418 + } 419 + 420 + if (reconfig) { 421 + ret = adf_dev_shutdown_cache_cfg(accel_dev); 422 + goto out; 423 + } 424 + 425 + adf_dev_stop(accel_dev); 426 + adf_dev_shutdown(accel_dev); 427 + 428 + out: 429 + mutex_unlock(&accel_dev->state_lock); 430 + return ret; 431 + } 432 + EXPORT_SYMBOL_GPL(adf_dev_down); 433 + 434 + int adf_dev_up(struct adf_accel_dev *accel_dev, bool config) 435 + { 436 + int ret = 0; 437 + 438 + if (!accel_dev) 439 + return -EINVAL; 440 + 441 + mutex_lock(&accel_dev->state_lock); 442 + 443 + if (adf_dev_started(accel_dev)) { 444 + dev_info(&GET_DEV(accel_dev), "Device qat_dev%d already up\n", 445 + accel_dev->accel_id); 446 + ret = -EALREADY; 447 + goto out; 448 + } 449 + 450 + if (config && GET_HW_DATA(accel_dev)->dev_config) { 451 + ret = GET_HW_DATA(accel_dev)->dev_config(accel_dev); 452 + if (unlikely(ret)) 453 + goto out; 454 + } 455 + 456 + ret = adf_dev_init(accel_dev); 457 + if (unlikely(ret)) 458 + goto out; 459 + 460 + ret = adf_dev_start(accel_dev); 461 + 462 + out: 463 + mutex_unlock(&accel_dev->state_lock); 464 + return ret; 465 + } 466 + EXPORT_SYMBOL_GPL(adf_dev_up);
+3 -20
drivers/crypto/qat/qat_common/adf_sysfs.c
··· 50 50 51 51 switch (ret) { 52 52 case DEV_DOWN: 53 - if (!adf_dev_started(accel_dev)) { 54 - dev_info(dev, "Device qat_dev%d already down\n", 55 - accel_id); 56 - return -EINVAL; 57 - } 58 - 59 53 dev_info(dev, "Stopping device qat_dev%d\n", accel_id); 60 54 61 - ret = adf_dev_shutdown_cache_cfg(accel_dev); 55 + ret = adf_dev_down(accel_dev, true); 62 56 if (ret < 0) 63 57 return -EINVAL; 64 58 65 59 break; 66 60 case DEV_UP: 67 - if (adf_dev_started(accel_dev)) { 68 - dev_info(dev, "Device qat_dev%d already up\n", 69 - accel_id); 70 - return -EINVAL; 71 - } 72 - 73 61 dev_info(dev, "Starting device qat_dev%d\n", accel_id); 74 62 75 - ret = GET_HW_DATA(accel_dev)->dev_config(accel_dev); 76 - if (!ret) 77 - ret = adf_dev_init(accel_dev); 78 - if (!ret) 79 - ret = adf_dev_start(accel_dev); 80 - 63 + ret = adf_dev_up(accel_dev, true); 81 64 if (ret < 0) { 82 65 dev_err(dev, "Failed to start device qat_dev%d\n", 83 66 accel_id); 84 - adf_dev_shutdown_cache_cfg(accel_dev); 67 + adf_dev_down(accel_dev, true); 85 68 return ret; 86 69 } 87 70 break;