Re: [PATCH] drm/nouveau/iccsense: fix memory leak in nvkm_iccsense_oneinit

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Branching on uninitialized memory triggers undefined behavior and potential out-of-bounds heap read.
--

--- Patch [1]: [PATCH] drm/nouveau/iccsense: fix memory leak in nvkm_iccsense_oneinit ---
commit db41910973f19f5f6b73007149784b3d74245e81
Author: Muhammad Bilal <meatuni001@gmail.com>

drm/nouveau/iccsense: fix memory leak in nvkm_iccsense_oneinit

This commit fixes a memory leak in nvkm_iccsense_oneinit() where the
stbl.rail array allocated by nvbios_iccsense_parse() was never freed.
The array is now correctly freed in a common exit path that handles both
success and error conditions.

> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> index 3ccdbbe2fad09..6bfe4913dcb66 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> @@ -239,6 +239,7 @@ nvkm_iccsense_oneinit(struct nvkm_subdev *subdev)
>  		return 0;
>  
>  	iccsense->data_valid = true;
> +	ret = 0;
>  	for (i = 0; i < stbl.nr_entry; ++i) {
>  		struct pwr_rail_t *pwr_rail = &stbl.rail[i];

[Severity: High]
This is a pre-existing issue, but could this read uninitialized memory?

In drivers/gpu/drm/nouveau/nvkm/subdev/bios/iccsense.c, the function
nvbios_iccsense_parse() allocates the rail array using kmalloc_objs(),
which does not zero memory:

    iccsense->rail = kmalloc_objs(struct pwr_rail_t, cnt);

Inside its parsing loop, if nvbios_extdev_parse() fails, it skips
initialization of the remaining fields:

    if (nvbios_extdev_parse(bios, rail->extdev_id, &extdev))
        continue;

This leaves rail->resistor_count and rail->resistors uninitialized.

Then, back here in nvkm_iccsense_oneinit(), we read
pwr_rail->resistor_count:

    if (pwr_rail->mode != 1 || !pwr_rail->resistor_count)
        continue;

If the uninitialized resistor_count happens to be non-zero, this loop
could execute using that garbage value and access memory out of bounds:

    for (r = 0; r < pwr_rail->resistor_count; ++r) {
        ...
        struct pwr_rail_resistor_t *res = &pwr_rail->resistors[r];

Should nvbios_iccsense_parse() zero the allocated memory to prevent this?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803215022.172201-1-meatuni001@gmail.com?part=1