Re: [PATCH v9 05/10] block: partitions: of: Attach partition fwnode to the block device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Fwnode reference leak in partition parsing error and skip paths.

Pre-existing issues:
- [High] Uninitialized stack variable `len` validation allows NULL pointer dereference when the 'reg' property is missing.
--

--- Patch [5]: [PATCH v9 05/10] block: partitions: of: Attach partition fwnode to the block device ---
commit a6e4cc997f016ffdca55d082b2ff10454aec24f8
Author: Loic Poulain <loic.poulain@oss.qualcomm.com>

block: partitions: of: Attach partition fwnode to the block device
    
The OF partition parser reads offset, size and label from each
"fixed-partitions" child node but does not associate that node with the
resulting partition block device. As a result a partition has no
of_node, unlike the whole-disk device which gets its firmware node via
add_disk_fwnode().

> diff --git a/block/partitions/of.c b/block/partitions/of.c
> index 53664ea06b654..4fc7c80c407de 100644
> --- a/block/partitions/of.c
> +++ b/block/partitions/of.c

[ ... ]

> @@ -62,6 +63,8 @@ static void add_of_partition(struct parsed_partitions *state, int slot,
>  		partname = of_get_property(np, "name", &len);
>  	strscpy(info->volname, partname, sizeof(info->volname));
>  
> +	state->parts[slot].fwnode = fwnode_handle_get(of_fwnode_handle(np));
> +
>  	seq_buf_printf(&state->pp_buf, "(%s)", info->volname);
>  }

[Severity: High]
Does this create a fwnode reference leak if the partitions are never
successfully added?

If blk_add_partitions() aborts early (such as for zoned block devices) or
skips a partition, it jumps to out_free_state and calls free_partitions().
This frees the parsed partitions array without dropping the unconsumed
fwnode references.

Also, if add_partition() fails before calling device_initialize():

block/partitions/core.c:add_partition() {
    ...
    err = -ENOMEM;
    bdev = bdev_alloc(disk, partno);
    if (!bdev)
        goto out_put_disk;
    ...
}

will it return an error without dropping the fwnode, since part_release()
will never be executed to clean it up?

[Severity: High]
This is a pre-existing issue, but can the uninitialized len variable cause a
NULL pointer dereference when parsing malformed device trees?

Looking at validate_of_partition() in the same file:

block/partitions/of.c:validate_of_partition() {
    int len;
    const __be32 *reg = of_get_property(np, "reg", &len);
    ...
}

If the 'reg' property is missing, of_get_property() returns NULL and leaves
the stack variable len uninitialized. If the uninitialized stack garbage in
len happens to satisfy the subsequent length check:

    if (len / sizeof(*reg) != a_cells + s_cells)

it will proceed to evaluate:

    offset = of_read_number(reg, a_cells);

Since reg is NULL, will this result in a kernel panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-block-as-nvmem-v9-0-f72935817dbf@oss.qualcomm.com?part=5