Re: [PATCH v4 2/5] nvmem: Add the Raspberry Pi OTP driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The driver exposes sensitive OTP memory (including device-specific private keys) with world-readable sysfs permissions.
- [High] Unchecked firmware response size modifying `fwbuf->count` under `__counted_by_le()` annotation triggers a fortified memcpy panic.
--

commit a8dbaf078fc585c391a3307f1adbf25d7433260e
Author: Gregor Herburger <gregor.herburger@linutronix.de>

nvmem: Add the Raspberry Pi OTP driver

Raspberry Pis have OTP registers which can be accessed through the videocore
firmware. Add a nvmem driver to support these OTP registers.

> diff --git a/drivers/nvmem/raspberrypi-otp.c b/drivers/nvmem/raspberrypi-otp.c
> new file mode 100644
> index 0000000000000..393640cb9e32b
> --- /dev/null
> +++ b/drivers/nvmem/raspberrypi-otp.c
> @@ -0,0 +1,130 @@
> +// SPDX-License-Identifier: GPL-2.0-only
[ ... ]
> +struct rpi_otp_header {
> +	__le32 start;
> +	__le32 count;
> +	__le32 data[] __counted_by_le(count);
> +};
> +
> +static int rpi_otp_read(void *context, unsigned int offset, void *buf, size_t bytes)
> +{
[ ... ]
> +	fwbuf->start = cpu_to_le32(offset / 4);
> +	fwbuf->count = cpu_to_le32(count);
> +
> +	ret = rpi_firmware_property(priv->fw, priv->read_tag, fwbuf,
> +				    sizeof(struct rpi_otp_header) + bytes);
> +	if (ret)
> +		goto out;
> +
> +	memcpy(buf, fwbuf->data, bytes);

If the firmware performs a short read or encounters a partial error, does it
update fwbuf->count to the actual number of words read?

If fwbuf->count is updated to a smaller value, the __counted_by_le() attribute
will dynamically shrink the bounded size of fwbuf->data. Could this cause the
memcpy() to trigger a fortified memcpy panic, since it uses the original
requested bytes size?

[ ... ]
> +static int rpi_otp_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct nvmem_device *nvmem;
> +	struct rpi_otp_priv *priv;
> +	const struct rpi_otp_driver_data *data;
> +	struct nvmem_config config = {
> +		.read_only = false,
> +		.word_size = 4,
> +		.stride = 4,
> +		.reg_read = rpi_otp_read,
> +		.reg_write = rpi_otp_write,
> +		.id = NVMEM_DEVID_NONE,
> +	};

The commit message notes this driver supports the "device specific private key
OTP (BCM2712 only)".

Does the nvmem subsystem default to creating world-readable sysfs attributes
unless explicitly restricted? Could this expose hardware private keys to
unprivileged users via sysfs?

Should the config set .root_only = true to prevent this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508-rpi-otp-driver-v4-0-cf8d725d8821@linutronix.de?part=2