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

Stefan Berger
2023-01-18T13:37:25Z

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;
> +}
> +