Re: [PATCH v3 4/6] pseries/plpks: add HCALLs for PKWM wrapping key life cycle management
flat view
From: sashiko-bot@kernel.org
Date: 2026-10-17 08:07:41
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] pseries/plpks: stale var->policy evaluated in plpks_is_wrapping_key_revoked() - [Low] powerpc/pseries: papr_hcalls.rst swaps H_PKS_GET_OBJECTLABELS output order -- commit 73a27f4e02c0030b70181101065300c5db3bc91b Author: Srish Srinivasan [off-list ref] pseries/plpks: add HCALLs for PKWM wrapping key life cycle management This commit implements the H_PKS_REVOKE_OBJECT, H_PKS_UNREVOKE_OBJECT, and H_PKS_GET_OBJECTLABELS hypervisor calls in the pseries PLPKS driver to support revoking, unrevoking, deleting, and listing PKWM wrapping keys.
quoted hunk ↗ jump to hunk
diff --git a/Documentation/arch/powerpc/papr_hcalls.rst b/Documentation/arch/powerpc/papr_hcalls.rst index 44c9c8b32ae3c..3455b403a0486 100644 --- a/Documentation/arch/powerpc/papr_hcalls.rst +++ b/Documentation/arch/powerpc/papr_hcalls.rst@@ -343,6 +343,39 @@ is returned to the caller.
[ ... ]
quoted hunk ↗ jump to hunk
+**H_PKS_GET_OBJECTLABELS** + +| Input: authorization, continueToken, out, outlen +| Out: *continue-token, number of object labels in the returned list, object + label list* +| Return Value: *H_Success, H_Function, H_State, H_Parameter, H_P2, H_P3, H_P4, + H_Authority, H_Busy, H_Aborted, H_Continue* + +H_PKS_GET_OBJECTLABELS is used to retrieve a list of object labels owned by the +specified consumer.
[Severity: Low]
Are the first two output fields for H_PKS_GET_OBJECTLABELS in
Documentation/arch/powerpc/papr_hcalls.rst swapped relative to the output
registers read in plpks_get_object_labels()?
In Documentation/arch/powerpc/papr_hcalls.rst, the Out field lists output
registers in order starting from r4 (retbuf[0]) and r5 (retbuf[1]), which
here places continue-token before the number of object labels.
However, in arch/powerpc/platforms/pseries/plpks.c:plpks_get_object_labels(),
retbuf[0] is read as labels_count and retbuf[1] is read as continuetoken:
continuetoken = retbuf[1];
labels_count = retbuf[0];
[ ... ]quoted hunk ↗ jump to hunk
diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c index a7953db3f23a6..edd189cfcc85c 100644 --- a/arch/powerpc/platforms/pseries/plpks.c +++ b/arch/powerpc/platforms/pseries/plpks.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -831,9 +846,6 @@ static int plpks_read_var(u8 consumer, struct plpks_var *var) if (var->namelen > PLPKS_MAX_NAME_SIZE) return -EINVAL; - if (var->policy & PLPKS_WRAPPINGKEY) - return -EPERM; - auth = construct_auth(consumer); if (IS_ERR(auth)) return PTR_ERR(auth);
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1189,6 +1218,419 @@ int plpks_unwrap_object(u8 **input_buf, u64 input_len, u8 **output_buf,
[ ... ]
quoted hunk ↗ jump to hunk
+int plpks_is_wrapping_key_revoked(struct plpks_var *var) +{ + int rc; + + if (!var || !var->name || !*var->name) + return -EINVAL; + + rc = plpks_read_var(PLPKS_OS_OWNER, var); + if (!rc) { + /* + * Reading a wrapping key is expected to fail with + * -EPERM and return its policy. + */ + rc = -EIO; + } else if (rc == -EPERM) { + if (var->policy & PLPKS_WRAPPINGKEY) { + if (var->policy & PLPKS_REVOKED) + rc = 1; + else + rc = 0; + } + } + + return rc; +} +EXPORT_SYMBOL_GPL(plpks_is_wrapping_key_revoked);
[Severity: Medium]
Can plpks_is_wrapping_key_revoked() evaluate a stale var->policy when
H_PKS_READ_OBJECT fails with H_AUTHORITY and retbuf[1] is 0, or overwrite
var->datalen and var->data if H_PKS_READ_OBJECT succeeds?
In arch/powerpc/platforms/pseries/plpks.c:plpks_read_var(), when
H_PKS_READ_OBJECT returns H_AUTHORITY (mapped to -EPERM), var->policy is only
updated if retbuf[1] is non-zero:
if (rc != H_SUCCESS) {
rc = pseries_status_to_err(rc);
if (rc != -EPERM || !retbuf[1])
goto out_free_output;
goto out_copy_policy;
}
Because plpks_is_wrapping_key_revoked() does not zero var->policy before
calling plpks_read_var(), an H_AUTHORITY failure with retbuf[1] == 0 leaves
var->policy untouched and plpks_is_wrapping_key_revoked() then checks the
pre-existing value in var->policy.
In the subsequent commit "pseries/plpks/wrapkey: expose PKWM wrapping key
management to userspace via sysfs", list_key_labels() defines a single
struct plpks_var var = {0} outside the loop and calls
plpks_is_wrapping_key_revoked(&var) across all labels without resetting
var.policy between iterations. Once an earlier iteration populates var.policy
with PLPKS_WRAPPINGKEY, any later iteration where H_PKS_READ_OBJECT returns
H_AUTHORITY with retbuf[1] == 0 will inspect the previous key's var.policy
and return 0 or 1 instead of -EPERM. The same happens if a caller of
plpks_is_wrapping_key_revoked() or plpks_del_wrapping_key() passes a
struct plpks_var with PLPKS_WRAPPINGKEY already set in var->policy.
Additionally, if H_PKS_READ_OBJECT unexpectedly succeeds on a readable
object, plpks_read_var() will overwrite var->datalen and copy into var->data
(if non-NULL) before plpks_is_wrapping_key_revoked() returns -EIO.
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261017074850.164031-1-ssrish@linux.ibm.com?part=4