Re: [PATCH v11 3/7] block: implement NVMEM provider

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] block: AB-BA deadlock and self-deadlock due to disk->open_mutex and NVMEM SRCU
- [Medium] block: delayed release of exclusive claim causes spurious -EBUSY on open
--

commit 163603c5c331585f01bddc0cceeabc4bb95a5455
Author: Loic Poulain <loic.poulain@oss.qualcomm.com>

block: implement NVMEM provider

This commit implements a block device NVMEM provider, allowing partitions
to expose factory-provisioned data like MAC addresses to consuming drivers.
The NVMEM accesses use BLK_OPEN_EXCL to keep other exclusive openers away
during the reads.

> diff --git a/block/blk-nvmem.c b/block/blk-nvmem.c
> new file mode 100644
> index 0000000000000..69b69184e1630
> --- /dev/null
> +++ b/block/blk-nvmem.c
[ ... ]
> +static int blk_nvmem_reg_read(void *priv, unsigned int from,
> +			      void *val, size_t bytes)
> +{
> +	struct block_device *bdev = priv;
> +	struct file *bdev_file;
> +	loff_t pos = from;
> +	ssize_t ret;
> +
> +	/* open and prevent other exclusive openers */
> +	bdev_file = bdev_file_open_by_dev(bdev->bd_dev,
> +					  BLK_OPEN_READ | BLK_OPEN_EXCL,
> +					  blk_nvmem_reg_read, NULL);
> +	if (IS_ERR(bdev_file))
> +		return PTR_ERR(bdev_file);
> +
> +	if (file_bdev(bdev_file) != bdev) {
> +		fput(bdev_file);
> +		return -ENODEV;
> +	}
> +
> +	ret = kernel_read(bdev_file, val, bytes, &pos);
> +	fput(bdev_file);

[Severity: Medium]
Will using fput() here and in the error path above defer the release of the
block device's exclusive claim?

Because fput() delegates the file release to task work or a workqueue, the
exclusive claim won't be yielded synchronously. If a process or script reads
the NVMEM cell and immediately tries to open the same block device
exclusively (e.g., to mount it), could it spuriously fail with -EBUSY?

Should this use bdev_fput() instead, which calls bdev_yield_claim()
synchronously before fput()?

> diff --git a/block/partitions/core.c b/block/partitions/core.c
> index eee954df1ac6e..c83227710584a 100644
> --- a/block/partitions/core.c
> +++ b/block/partitions/core.c
[ ... ]
> @@ -289,6 +289,8 @@ void drop_partition(struct block_device *part)
>  	xa_erase(&part->bd_disk->part_tbl, bdev_partno(part));
>  	kobject_put(part->bd_holder_dir);
>  
> +	blk_nvmem_del(part);

[Severity: High]
Can this create an AB-BA deadlock with concurrent NVMEM readers?

drop_partition() is called with disk->open_mutex held. blk_nvmem_del() calls
nvmem_unregister(), which waits for active readers via
synchronize_srcu(&nvmem->srcu).

If an untrusted user space process is reading the NVMEM sysfs file while
the block device is being removed, the concurrent reader executing
__nvmem_reg_read() holds srcu_read_lock(&nvmem->srcu) and calls
bdev_file_open_by_dev(), which blocks on disk->open_mutex.

Won't the teardown thread and the reader thread deadlock waiting for
each other's locks?

[ ... ]
> @@ -406,6 +408,9 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,
>  	/* suppress uevent if the disk suppresses it */
>  	if (!dev_get_uevent_suppress(ddev))
>  		kobject_uevent(&pdev->kobj, KOBJ_ADD);
> +
> +	blk_nvmem_add(bdev);

[Severity: High]
Could this result in a self-deadlock during partition creation?

add_partition() executes with disk->open_mutex held. If the NVMEM device
requires layout parsing (e.g., reading ONIE TLV data), the NVMEM core
will read from the device during nvmem_register() via __nvmem_reg_read().

This invokes blk_nvmem_reg_read(), which opens the block device via
bdev_file_open_by_dev() and blocks attempting to acquire the already held
disk->open_mutex.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-block-as-nvmem-v11-0-f890ac74f87d@oss.qualcomm.com?part=3