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 <[email protected]>

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/[email protected]?part=1

Reply via email to