On 17.09.20 17:01, Ingo Franzki wrote:

On 15.09.2020 11:21, Harald Freudenberger wrote:

...

When both the paes and the pkey kernel module are statically build into the kernel, the paes cipher selftests run before the pkey kernel module is initialized. So a static variable set in the pkey init function and used in the pkey_clr2protkey function is not initialized when the paes cipher's selftests request to call pckmo for transforming a clear key value into a protected key.

This patch moves the initial setup of the static variable into the function pck_clr2protkey. So it's possible, to use the function for transforming a clear to a protected key even before the pkey init function has been called and the paes selftests may run successful.

Signed-off-by: Harald Freudenberger freude@linux.ibm.com Reported-by: Alexander Egorenkov Alexander.Egorenkov@ibm.com Cc: Stable stable@vger.kernel.org

---

drivers/s390/crypto/pkey_api.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/s390/crypto/pkey_api.c b/drivers/s390/crypto/pkey_api.c index 490917cd44d0..5f75c9dfe071 100644 --- a/drivers/s390/crypto/pkey_api.c +++ b/drivers/s390/crypto/pkey_api.c @@ -34,9 +34,6 @@ MODULE_DESCRIPTION("s390 protected key interface"); #define KEYBLOBBUFSIZE 8192 /* key buffer size used for internal processing */ #define MAXAPQNSINLIST 64 /* max 64 apqns within a apqn list */ -/* mask of available pckmo subfunctions, fetched once at module init */ -static cpacf_mask_t pckmo_functions;

/*

• debug feature data and functions

*/ @@ -90,6 +87,9 @@ static int pkey_clr2protkey(u32 keytype, const struct pkey_clrkey *clrkey, struct pkey_protkey *protkey) {

• /* mask of available pckmo subfunctions */
• static cpacf_mask_t pckmo_functions;
• long fc; int keysize; u8 paramblock[64];

@@ -113,11 +113,13 @@ static int pkey_clr2protkey(u32 keytype, return -EINVAL; }

• /*

• * Check if the needed pckmo subfunction is available.
  

• * These subfunctions can be enabled/disabled by customers
  

• * in the LPAR profile or may even change on the fly.
  

• */
  

• /* did we already check for PCKMO ? */
• if (!pckmo_functions.bytes[0]) {

• /* no, so check now */
  

• if (!cpacf_query(CPACF_PCKMO, &pckmo_functions))
  

• 	return -ENODEV;
  

• }

Does this need a lock here? What if 2 processes or threads call this concurrently? Certainly the cpacf_query will produce the same result, but updating static pckmo_functions concurrently might cause problems.

I'd say as long as both concurrent threads will fetch the very same value - No, even if the update is not done atomically. For example:   u64 blubber[2] = { 0, 0 };   // thread 1 updates blubber but unfortunately not in an atomic way:   blubber[0] = 0x1122334455667788;   // now interrupted by thread 2 which updates blubber now:   blubber[1] = 0x8877665544332211;   blubber[0] = 0x1122334455667788;   // and finally thread1 comes again and does the 2. half of the update:   blubber[1] = 0x8877665544332211; even if you reshuffle this the result is the very same as long as both threads update blubber WITH THE SAME VALUE. For a different value, I do agree some kind of locking is needed. At least this is my understanding...

...

• /* check for the pckmo subfunction we need now */ if (!cpacf_test_func(&pckmo_functions, fc)) { DEBUG_ERR("%s pckmo functions not available\n", __func__); return -ENODEV;

@@ -1853,7 +1855,7 @@ static struct miscdevice pkey_dev = { */ static int __init pkey_init(void) {

• cpacf_mask_t kmc_functions;

• cpacf_mask_t func_mask;

/* * The pckmo instruction should be available - even if we don't @@ -1861,15 +1863,15 @@ static int __init pkey_init(void) * is also the minimum level for the kmc instructions which * are able to work with protected keys. */

• if (!cpacf_query(CPACF_PCKMO, &pckmo_functions))

• if (!cpacf_query(CPACF_PCKMO, &func_mask)) return -ENODEV;

/* check for kmc instructions available */

• if (!cpacf_query(CPACF_KMC, &kmc_functions))

• if (!cpacf_query(CPACF_KMC, &func_mask)) return -ENODEV;

• if (!cpacf_test_func(&kmc_functions, CPACF_KMC_PAES_128) ||

•    !cpacf_test_func(&kmc_functions, CPACF_KMC_PAES_192) ||
  

•    !cpacf_test_func(&kmc_functions, CPACF_KMC_PAES_256))
  

• if (!cpacf_test_func(&func_mask, CPACF_KMC_PAES_128) ||

•    !cpacf_test_func(&func_mask, CPACF_KMC_PAES_192) ||
  

•    !cpacf_test_func(&func_mask, CPACF_KMC_PAES_256))
  

  return -ENODEV;

pkey_debug_init();