Re: [PATCH v3 22/24] powerpc/pseries: Implement secvars for dynamic secure boot

[Stefan Berger]

On 1/18/23 01:10, Andrew Donnellan wrote:

> +
> +// PLPKS dynamic secure boot doesn't give us a format string in the same way OPAL does.
> +// Instead, report the format using the SB_VERSION variable in the keystore.
> +static ssize_t plpks_secvar_format(char *buf)

Ideally there would be a size_t accompanying this buffer...

> +{
> +	struct plpks_var var = {0};
> +	ssize_t ret;
> +
> +	var.component = NULL;
> +	// Only the signed variables have null bytes in their names, this one doesn't
> +	var.name = "SB_VERSION";
> +	var.namelen = 10;
> +	var.datalen = 1;
> +	var.data = kzalloc(1, GFP_KERNEL);

NULL pointer check?

> +
> +	// Unlike the other vars, SB_VERSION is owned by firmware instead of the OS
> +	ret = plpks_read_fw_var(&var);
> +	if (ret) {
> +		if (ret == -ENOENT) {
> +			ret = snprintf(buf, SECVAR_MAX_FORMAT_LEN, "ibm,plpks-sb-unknown");
> +		} else {
> +			pr_err("Error %ld reading SB_VERSION from firmware\n", ret);
> +			ret = -EIO;
> +		}
> +		goto err;
> +	}
> +
> +	// This string is made up by us - the hypervisor doesn't provide us
> +	// with a format string in the way that OPAL firmware does. Hypervisor
> +	// defines SB_VERSION as a "1 byte unsigned integer value".
> +	ret = snprintf(buf, SECVAR_MAX_FORMAT_LEN, "ibm,plpks-sb-v%hhu", var.data[0]);
> +
> +err:
> +	kfree(var.data);
> +	return ret;
> +}
> +