Re: [PATCH] staging: nvec: validate battery response length before memcpy

[Marc Dietrich]

Hello Sebastian,

On Tue, 24 Mar 2026, Sebastian Josue Alba Vives wrote:

> In nvec_power_notifier(), the response length from the embedded
> controller is used directly as the size argument to memcpy() when
> copying battery manufacturer, model, and type strings. The
> destination buffers (bat_manu, bat_model, bat_type) are fixed at 30
> bytes, but res->length is a u8 that can be up to 255, allowing a
> heap buffer overflow.
>
> Additionally, if res->length is less than 2, the subtraction
> res->length - 2 wraps around as an unsigned value, resulting in a
> large copy that corrupts kernel heap memory.
>
> Add bounds checks before each memcpy to ensure the copy length does
> not exceed the destination buffer size, and that res->length is at
> least 2 to prevent unsigned integer underflow.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Sebastian Josue Alba Vives <sebasjosue84@xxxxxxxxx>
> ---
> drivers/staging/nvec/nvec_power.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/staging/nvec/nvec_power.c b/drivers/staging/nvec/nvec_power.c
> index 2faab9fde..29beef0a7 100644
> --- a/drivers/staging/nvec/nvec_power.c
> +++ b/drivers/staging/nvec/nvec_power.c
> @@ -193,14 +193,20 @@ static int nvec_power_bat_notifier(struct notifier_block *nb,
> 		power->bat_temperature = res->plu - 2732;
> 		break;
> 	case MANUFACTURER:
> +		if (res->length < 2 || res->length - 2 > sizeof(power->bat_manu) - 1)
> +			break;

I think this check is a valid one, even if unlikely to fail. Could be a
bit nicer by replacing sizeof() with a contant and storing res->length-2 
to some variable, but I guess that's a matter of taste.

Tested-by: Marc Dietrich <marvin24@xxxxxx>

Thanks,

Marc

> 		memcpy(power->bat_manu, &res->plc, res->length - 2);
> 		power->bat_manu[res->length - 2] = '\0';
> 		break;
> 	case MODEL:
> +		if (res->length < 2 || res->length - 2 > sizeof(power->bat_model) - 1)
> +			break;
> 		memcpy(power->bat_model, &res->plc, res->length - 2);
> 		power->bat_model[res->length - 2] = '\0';
> 		break;
> 	case TYPE:
> +		if (res->length < 2 || res->length - 2 > sizeof(power->bat_type) - 1)
> +			break;
> 		memcpy(power->bat_type, &res->plc, res->length - 2);
> 		power->bat_type[res->length - 2] = '\0';
> 		/*
> --
> 2.43.0