Re: [PATCH v4 9/9] pseries/plpks/wrapkey: expose PKWM wrapping key management to userspace via sysfs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] powerpc/pseries/plpks: use-after-free of plpks_kobj in plpks_init_child_kobj()
- [Medium] powerpc/pseries: wrapkey_sysfs_init() does not ensure default key exists
--

commit 81ec5557b710c94f51afed01c64f4716ffcf0140
Author: Srish Srinivasan <ssrish@linux.ibm.com>

pseries/plpks/wrapkey: expose PKWM wrapping key management to userspace via sysfs

This commit adds a sysfs interface under /sys/firmware/plpks/wrapkey to allow
userspace to create, revoke, unrevoke, delete, and list PKWM wrapping keys by
label.

> diff --git a/Documentation/ABI/testing/sysfs-firmware-plpks b/Documentation/ABI/testing/sysfs-firmware-plpks
> index cba061e4eee2a..4144e868da1b1 100644
> --- a/Documentation/ABI/testing/sysfs-firmware-plpks
> +++ b/Documentation/ABI/testing/sysfs-firmware-plpks

[ ... ]

> +What:		/sys/firmware/plpks/wrapkey/list_active
> +Date:		August 2026
> +Contact:	Srish Srinivasan <ssrish@linux.ibm.com>
> +Description:	(RO) List the labels of active, non-revoked wrapping keys. The
> +		default wrapping key, named "default-wrapping-key", is always
> +		included in this list.
> +
> +		For example:
> +
> +			cat /sys/firmware/plpks/wrapkey/list_active

[ ... ]

> diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
> index 7d125e288f6ef..14d2eed4d881a 100644
> --- a/arch/powerpc/platforms/pseries/Kconfig
> +++ b/arch/powerpc/platforms/pseries/Kconfig
> @@ -215,3 +215,18 @@ config PPC_SVM
>  	 those guests.
>  
>  	 If unsure, say "N".
> +
> +config PLPKS_WRAPKEY_SYSFS
> +	bool "Enable sysfs interface for managing PKWM wrapping keys"
> +	default n
> +	depends on PSERIES_PLPKS
> +	depends on SYSFS
> +	depends on TRUSTED_KEYS_PKWM
> +	help
> +	  The PKWM trusted source supports user-created wrapping keys that are
> +	  managed by the OS. These wrapping keys can be created, revoked,
> +	  unrevoked, and deleted through sysfs. Their labels can also be viewed
> +	  through sysfs.
> +
> +	  Say Y if PLPKS and PKWM are enabled and you want to isolate trusted
> +	  keys using user-created wrapping keys.

[ ... ]

> diff --git a/arch/powerpc/platforms/pseries/plpks-sysfs.c b/arch/powerpc/platforms/pseries/plpks-sysfs.c
> index f2436229f3233..0ca13bb6405f8 100644
> --- a/arch/powerpc/platforms/pseries/plpks-sysfs.c
> +++ b/arch/powerpc/platforms/pseries/plpks-sysfs.c
> @@ -55,6 +55,17 @@ static umode_t plpks_config_attr_is_visible(struct kobject *kobj,
>  	return attr->mode;
>  }
>  
> +int plpks_init_child_kobj(struct kobject *kobj, const struct kobj_type *ktype,
> +			  const char *name)
> +{
> +	kobject_init(kobj, ktype);
> +
> +	if (!plpks_kobj)
> +		return -ENODEV;
> +
> +	return kobject_add(kobj, plpks_kobj, "%s", name);
> +}
> +
>  int plpks_config_create_softlink(struct kobject *from)
>  {
>  	if (!plpks_config_kobj)

[Severity: Medium]
Can plpks_init_child_kobj() dereference a freed plpks_kobj if
plpks_sysfs_init() fails after creating plpks_kobj?

In arch/powerpc/platforms/pseries/plpks-sysfs.c:plpks_sysfs_init():

	plpks_config_kobj = kobject_create_and_add("config", plpks_kobj);
	if (!plpks_config_kobj) {
		pr_err("Failed to create plpks config kobj\n");
		kobject_put(plpks_kobj);
		return -ENOMEM;
	}

	rc = plpks_sysfs_config(plpks_config_kobj);
	if (rc) {
		pr_err("Failed to create attribute group for plpks config\n");
		kobject_put(plpks_config_kobj);
		kobject_put(plpks_kobj);
		return rc;
	}

If either of those steps fails during machine_subsys_initcall, kobject_put()
frees plpks_kobj without resetting the static pointer to NULL. When
wrapkey_sysfs_init() later runs at machine_late_initcall and calls
plpks_init_child_kobj(), the !plpks_kobj check passes and kobject_add()
operates on the dangling plpks_kobj pointer.

> diff --git a/arch/powerpc/platforms/pseries/plpks-wrapkey-sysfs.c b/arch/powerpc/platforms/pseries/plpks-wrapkey-sysfs.c
> new file mode 100644
> index 0000000000000..d810d6d64fb92
> --- /dev/null
> +++ b/arch/powerpc/platforms/pseries/plpks-wrapkey-sysfs.c

[ ... ]

> +static __init int wrapkey_sysfs_init(void)
> +{
> +	int rc;
> +
> +	if (!plpks_revoke_is_supported()) {
> +		pr_err("H_PKS_{UN}REVOKE_OBJECT interface not supported\n");
> +		return -ENODEV;
> +	}
> +
> +	wrapkey_kobj = kzalloc_obj(*wrapkey_kobj);
> +	if (!wrapkey_kobj)
> +		return -ENOMEM;
> +
> +	rc = plpks_init_child_kobj(wrapkey_kobj, &wrapkey_ktype, "wrapkey");
> +
> +	if (rc) {
> +		kobject_put(wrapkey_kobj);
> +		wrapkey_kobj = NULL;
> +	}
> +
> +	return rc;
> +}
> +
> +machine_late_initcall(pseries, wrapkey_sysfs_init);

[Severity: Medium]
Is "default-wrapping-key" guaranteed to exist in PLPKS when
wrapkey_sysfs_init() exposes /sys/firmware/plpks/wrapkey?

Both the commit message and Documentation/ABI/testing/sysfs-firmware-plpks
state that "default-wrapping-key" is always included in list_active, and
create_store() rejects "default-wrapping-key" with -EEXIST.

However, "default-wrapping-key" is only generated in
security/keys/trusted-keys/trusted_pkwm.c:trusted_pkwm_init():

	var = (struct plpks_var) {
		.name = (u8 *)PLPKS_DEFAULT_WRAPKEY_LABEL,
		.namelen = sizeof(PLPKS_DEFAULT_WRAPKEY_LABEL) - 1,
		.policy = PLPKS_WRAPPINGKEY,
		.os = PLPKS_VAR_LINUX,
		.component = PLPKS_WRAPKEY_COMPONENT
	};

	ret = plpks_gen_wrapping_key(&var);

Because CONFIG_PLPKS_WRAPKEY_SYSFS is a bool while CONFIG_TRUSTED_KEYS is
tristate, wrapkey_sysfs_init() runs at machine_late_initcall even when
trusted.ko has not been loaded, or when init_trusted() selects another
trusted key source (such as TPM) earlier in trusted_key_sources[]. On a fresh
LPAR where trusted_pkwm_init() has not run, won't reading list_active omit
"default-wrapping-key" while writing "default-wrapping-key" to create fails
with -EEXIST?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017183853.596052-1-ssrish@linux.ibm.com?part=9