Re: [PATCH] staging: nvec: fix out-of-bounds string copies

[Greg KH]

On Thu, Jul 30, 2026 at 11:12:20AM +0700, Dang Vu Duc Hien wrote:
> The function `nvec_power_bat_notifier()` assumes that `res->length` is
> both >= 2 and within the battery info buffers.
> Technically, `res->length` is provided by the embedded controller (EC)
> and is unverified (ie. a malformed response). If `res->length` < 2, the
> expression `res->length - 2` evaluates to a massive size_t value which
> is passed to `memcpy()`, causing severe memory corruption. Conversely,
> `res->length - 2` being larger than the buffer also causes an
> out-of-bounds write.
> 
> Introduce helper function `nvec_bat_str_copy()` to safely handle these
> string copies by:
> 1. Ensuring that it returns an error to the upper layer along with a
>    `dev_warn()` message when `res->length` < 2.
> 2. Ensuring that it provides toleration by copying as much as it can
>    along with a `dev_warn()` when `res->length - 2` is too big.
> 3. Guaranteeing a properly sized, null-terminated destination string if
>    the copy succeeds.
> 
> Signed-off-by: Dang Vu Duc Hien <dvdh12707@xxxxxxxxx>
> ---
> Note: Coincidentally, I have been working on this exact issue earlier
> and had this patch prepared just before Lucas Jeffrey submitted his RFC.
> I am sending it here to answer his protocol validation question with code.
> ---
>  drivers/staging/nvec/nvec_power.c | 41 ++++++++++++++++++++++++++-----
>  1 file changed, 35 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/nvec/nvec_power.c b/drivers/staging/nvec/nvec_power.c
> index c514d51a9e57..12699515356d 100644
> --- a/drivers/staging/nvec/nvec_power.c
> +++ b/drivers/staging/nvec/nvec_power.c
> @@ -117,6 +117,32 @@ static void get_bat_mfg_data(struct nvec_power *power)
>  	}
>  }
>  
> +static int nvec_bat_str_copy(struct nvec_power *power,
> +			     struct bat_response *res, char *dst,
> +			     unsigned int dst_size, const char *prop_name)
> +{
> +	unsigned int len;
> +
> +	if (res->length < 2) {
> +		dev_warn(power->nvec->dev,
> +			 "battery %s response length too short (%u)\n",
> +			 prop_name, res->length);

So you are allowing a malicious device to spam the kernel log?  If you
really worry about malicious devices, this isn't a good idea :)

But this is a platform device, built into the hardware, so you have to
assume that it is NOT malicious and we can trust it, as we trust all
platform devices these days.  If we wish to change that model, then we
need to do more work on Linux than just stuff like this.

thanks,

greg k-h