Re: [PATCH v2 1/2] libnvdimm/labels: Prevent integer overflow in __nd_label_validate()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The expression `1 << (7 + nsindex[i]->labelsize)` invokes C undefined behavior if the untrusted `nsindex[i]->labelsize` is maliciously large.
--

commit 1590d4e94c2716631030a170a8e554c454f824c4
Author: Bryam Vargas <hexlabsecurity@proton.me>

libnvdimm/labels: Prevent integer overflow in __nd_label_validate()

This commit fixes an out-of-bounds write in nd_label_data_init() by
evaluating the product of nslot and label_size as a 64-bit value. This
prevents a crafted nslot from bypassing the config area bounds check
due to a 32-bit integer overflow.

> diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
> index 4218e3ac4a2ab..ec12ce72cfe2c 100644
> --- a/drivers/nvdimm/label.c
> +++ b/drivers/nvdimm/label.c

[Severity: High]
This is a pre-existing issue, but while looking at __nd_label_validate(),
is it possible for the labelsize shift to invoke undefined behavior?

The labelsize field is read directly from the untrusted label storage
medium:

drivers/nvdimm/label.c:__nd_label_validate() {
        ...
        if (version >= 102)
                labelsize = 1 << (7 + nsindex[i]->labelsize);
        else
        ...
}

If a malicious or corrupted medium provides a value of 25 or greater for
nsindex[i]->labelsize, 7 + labelsize evaluates to 32 or more. Because 1
is a 32-bit signed integer, shifting it by 31 bits causes a signed
integer overflow, and shifting by 32 or more bits exceeds the type width.

Could this trigger a kernel panic on systems with undefined behavior
sanitizers enabled, or lead to unpredictable behavior due to compiler
optimizations?

> @@ -202,7 +202,7 @@ static int __nd_label_validate(struct nvdimm_drvdata *ndd)
>  		}
>  
>  		nslot = __le32_to_cpu(nsindex[i]->nslot);
> -		if (nslot * sizeof_namespace_label(ndd)
> +		if ((u64)nslot * sizeof_namespace_label(ndd)
>  				+ 2 * sizeof_namespace_index(ndd)
>  				> ndd->nsarea.config_size) {
>  			dev_dbg(dev, "nsindex%d nslot: %u invalid, config_size: %#x\n",

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623-b4-disp-1f2c537a-v2-0-59af73f1f090@proton.me?part=1