Re: [PATCH v5 9/9] arm64: dts: qcom: arduino-imola: Describe NVMEM layout for WiFi/BT addresses

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Confused Deputy / Use-After-Free via dev_t reuse in block NVMEM provider.
--

commit 459b659c960efdc8a9e2177c5bc4f832e7ee5730
Author: Daniel Golle <daniel@makrotopia.org>

block: implement NVMEM provider

On embedded devices using an eMMC it is common that one or more partitions
on the eMMC are used to store MAC addresses and Wi-Fi calibration EEPROM
data. Allow referencing the partition in device tree for the kernel and
Wi-Fi drivers accessing it via the NVMEM layer. For now, NVMEM is only
registered for the whole disk block device.

> diff --git a/block/blk-nvmem.c b/block/blk-nvmem.c
> new file mode 100644
> index 0000000000000..c005f059d9fe5
> --- /dev/null
> +++ b/block/blk-nvmem.c
[ ... ]
> +static int blk_nvmem_reg_read(void *priv, unsigned int from, void *val, size_t bytes)
> +{
> +	blk_mode_t mode = BLK_OPEN_READ | BLK_OPEN_RESTRICT_WRITES;
> +	dev_t devt = (dev_t)(uintptr_t)priv;
> +	size_t bytes_left = bytes;
> +	loff_t pos = from;
> +	int ret = 0;
> +
> +	struct file *bdev_file __free(fput) = bdev_file_open_by_dev(devt, mode, priv, NULL);

[Severity: High]
Does this code allow for a cross-device data leak if the block device is
removed and its dev_t is reassigned?

If an NVMEM consumer (such as a WiFi driver or a process holding an open
sysfs file) retains an active reference to the NVMEM cell, the nvmem_device
remains alive even after blk_nvmem_del() calls nvmem_unregister() because
nvmem_unregister() only decrements the registration reference count.

If the block layer then frees the removed device's dev_t, it could be
immediately reassigned to a newly plugged block device (e.g. a USB stick).

If the consumer later reads the NVMEM cell, wouldn't bdev_file_open_by_dev()
successfully open the new device using the reused dev_t, allowing the
consumer to unknowingly read data from the unrelated device?

> +	if (IS_ERR(bdev_file))
> +		return PTR_ERR(bdev_file);
> +
[ ... ]
> +void blk_nvmem_add(struct block_device *bdev)
> +{
> +	struct device *dev = &bdev->bd_device;
> +	struct nvmem_config config = {};
[ ... ]
> +	config.id = NVMEM_DEVID_NONE;
> +	config.dev = dev;
> +	config.name = dev_name(dev);
> +	config.owner = THIS_MODULE;
> +	config.priv = (void *)(uintptr_t)dev->devt;

[Severity: High]
Is it safe to store the dev_t here as the priv pointer?

Since the dev_t identifier can be reassigned by the block layer once the
device is removed, storing it this way might lead to the isolation issue
mentioned above if an NVMEM consumer holds a reference past the device's
removal.

> +	config.reg_read = blk_nvmem_reg_read;
> +	config.size = bdev_nr_bytes(bdev);
> +	config.word_size = 1;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-block-as-nvmem-v5-0-95e0b30fff90@oss.qualcomm.com?part=9