On Wed, 2024-01-24 at 20:10 +0000, Verma, Vishal L wrote:

On Wed, 2024-01-24 at 14:15 -0500, Mimi Zohar wrote:

quoted

On Wed, 2024-01-24 at 18:21 +0000, Verma, Vishal L wrote:

quoted

On Wed, 2023-11-08 at 07:36 +0000, Chen Ni wrote:

quoted

Add check for strsep() in order to transfer the error.

Fixes: cd3bc044af48 ("KEYS: encrypted: Instantiate key with user-
provided decrypted data")
Signed-off-by: Chen Ni <redacted>
---
 security/keys/encrypted-keys/encrypted.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/security/keys/encrypted-keys/encrypted.c

b/security/keys/encrypted-keys/encrypted.c
index 8af2136069d2..76f55dd13cb8 100644

--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c

@@ -237,6 +237,10 @@ static int datablob_parse(char *datablob, const

char **format,
 			break;
 		}
 		*decrypted_data = strsep(&datablob, " \t");
+		if (!*decrypted_data) {
+			pr_info("encrypted_key: decrypted_data is
missing\n");
+			break;
+		}

Hello,

This patch seems to break keyring usage in CXL and NVDIMM, with the
"decrypted_data is missing" error path being hit. Reverting this commit
fixes the tests. I'm not sure if there are valid scenarios where this is
expected to be empty?

Here's an strace snippet of where the error occurs:

   keyctl(KEYCTL_SEARCH, KEY_SPEC_USER_KEYRING, "user", "nvdimm-master", 0) = 76300785
   openat(AT_FDCWD, "/sys/devices/platform/cxl_acpi.0/root0/nvdimm-bridge0/ndbus0/nmem0/state", O_RDONLY|O_CLOEXEC) = 3
   read(3, "idle\n", 1024)                 = 5
   close(3)                                = 0
   keyctl(KEYCTL_SEARCH, KEY_SPEC_USER_KEYRING, "encrypted", "nvdimm:0", 0) = -1 ENOKEY (Required key not available)
   uname({sysname="Linux", nodename="fedora", ...}) = 0
   newfstatat(AT_FDCWD, "/etc/ndctl/keys/nvdimm_0_fedora.blob", 0x7fff23fbc210, 0) = -1 ENOENT (No such file or directory)
   add_key("encrypted", "nvdimm:0", "new enc32 user:nvdimm-master 32", 31, KEY_SPEC_USER_KEYRING) = -1 EINVAL (Invalid argument)

Indeed!  The user-provided decrypted data should be optional.   The change needs
to be reverted.

Ah, thanks for confirming! Would you like me to send a revert patch or
will you do it?

Revert "KEYS: encrypted: Add check for strsep"
    
This reverts commit b4af096b5df5dd131ab796c79cedc7069d8f4882.
    
New encrypted keys are created either from kernel-generated random
numbers or user-provided decrypted data.  Revert the change requiring
user-provided decrypted data.


Can I add your Reported-by?

Mimi