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

Sebastian Josue Alba Vives posted 1 patch 2 days, 23 hours ago
There is a newer version of this series
drivers/staging/nvec/nvec_power.c | 41 +++++++++++++++++++++----------
1 file changed, 28 insertions(+), 13 deletions(-)
[PATCH v2] staging: nvec: validate battery response length before memcpy
Posted by Sebastian Josue Alba Vives 2 days, 23 hours ago
From: Sebastián Alba Vives <sebasjosue84@gmail.com>

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.

Introduce NVEC_BAT_STRING_SIZE to replace the hardcoded buffer
size, store res->length - 2 in a local copy_len variable for
clarity, and add bounds checks before each memcpy to ensure the
copy length does not exceed the destination buffer and that
res->length is at least 2 to prevent unsigned integer underflow.

Tested-by: Marc Dietrich <marvin24@gmx.de>
Cc: stable@vger.kernel.org
Signed-off-by: Sebastián Alba Vives <sebasjosue84@gmail.com>
---
v2:
  - Introduce NVEC_BAT_STRING_SIZE constant to replace hardcoded
    buffer size (Marc Dietrich)
  - Store res->length - 2 in local copy_len variable for clarity
    (Marc Dietrich)
  - Use NVEC_BAT_STRING_SIZE in strncmp call for consistency
 drivers/staging/nvec/nvec_power.c | 41 +++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/nvec/nvec_power.c b/drivers/staging/nvec/nvec_power.c
index 2faab9fde..7b7980127 100644
--- a/drivers/staging/nvec/nvec_power.c
+++ b/drivers/staging/nvec/nvec_power.c
@@ -19,6 +19,7 @@
 #include "nvec.h"
 
 #define GET_SYSTEM_STATUS 0x00
+#define NVEC_BAT_STRING_SIZE 30
 
 struct nvec_power {
 	struct notifier_block notifier;
@@ -38,9 +39,9 @@ struct nvec_power {
 	int bat_temperature;
 	int bat_cap;
 	int bat_type_enum;
-	char bat_manu[30];
-	char bat_model[30];
-	char bat_type[30];
+	char bat_manu[NVEC_BAT_STRING_SIZE];
+	char bat_model[NVEC_BAT_STRING_SIZE];
+	char bat_type[NVEC_BAT_STRING_SIZE];
 };
 
 enum {
@@ -192,22 +193,36 @@ static int nvec_power_bat_notifier(struct notifier_block *nb,
 	case TEMPERATURE:
 		power->bat_temperature = res->plu - 2732;
 		break;
-	case MANUFACTURER:
-		memcpy(power->bat_manu, &res->plc, res->length - 2);
-		power->bat_manu[res->length - 2] = '\0';
+	case MANUFACTURER: {
+		size_t copy_len = res->length - 2;
+
+		if (res->length < 2 || copy_len > NVEC_BAT_STRING_SIZE - 1)
+			break;
+		memcpy(power->bat_manu, &res->plc, copy_len);
+		power->bat_manu[copy_len] = '\0';
 		break;
-	case MODEL:
-		memcpy(power->bat_model, &res->plc, res->length - 2);
-		power->bat_model[res->length - 2] = '\0';
+	}
+	case MODEL: {
+		size_t copy_len = res->length - 2;
+
+		if (res->length < 2 || copy_len > NVEC_BAT_STRING_SIZE - 1)
+			break;
+		memcpy(power->bat_model, &res->plc, copy_len);
+		power->bat_model[copy_len] = '\0';
 		break;
-	case TYPE:
-		memcpy(power->bat_type, &res->plc, res->length - 2);
-		power->bat_type[res->length - 2] = '\0';
+	}
+	case TYPE: {
+		size_t copy_len = res->length - 2;
+
+		if (res->length < 2 || copy_len > NVEC_BAT_STRING_SIZE - 1)
+			break;
+		memcpy(power->bat_type, &res->plc, copy_len);
+		power->bat_type[copy_len] = '\0';
 		/*
 		 * This differs a little from the spec fill in more if you find
 		 * some.
 		 */
-		if (!strncmp(power->bat_type, "Li", 30))
+		if (!strncmp(power->bat_type, "Li", NVEC_BAT_STRING_SIZE))
 			power->bat_type_enum = POWER_SUPPLY_TECHNOLOGY_LION;
 		else
 			power->bat_type_enum = POWER_SUPPLY_TECHNOLOGY_UNKNOWN;
-- 
2.43.0

Re: [PATCH v2] staging: nvec: validate battery response length before memcpy
Posted by kernel test robot 2 days, 7 hours ago
Hi Sebastian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on staging/staging-testing]

url:    https://github.com/intel-lab-lkp/linux/commits/Sebastian-Josue-Alba-Vives/staging-nvec-validate-battery-response-length-before-memcpy/20260330-174322
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20260330060926.751031-1-sebasjosue84%40gmail.com
patch subject: [PATCH v2] staging: nvec: validate battery response length before memcpy
config: arm64-randconfig-r054-20260331 (https://download.01.org/0day-ci/archive/20260331/202603310649.6iHw5wAQ-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 9.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260331/202603310649.6iHw5wAQ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603310649.6iHw5wAQ-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/staging/nvec/nvec_power.c: In function 'nvec_power_bat_notifier':
   drivers/staging/nvec/nvec_power.c:237:12: error: invalid storage class for function 'nvec_power_get_property'
     237 | static int nvec_power_get_property(struct power_supply *psy,
         |            ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/nvec/nvec_power.c:253:12: error: invalid storage class for function 'nvec_battery_get_property'
     253 | static int nvec_battery_get_property(struct power_supply *psy,
         |            ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/nvec/nvec_power.c:344:18: error: initializer element is not constant
     344 |  .get_property = nvec_battery_get_property,
         |                  ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/nvec/nvec_power.c:344:18: note: (near initialization for 'nvec_bat_psy_desc.get_property')
   drivers/staging/nvec/nvec_power.c:352:18: error: initializer element is not constant
     352 |  .get_property = nvec_power_get_property,
         |                  ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/staging/nvec/nvec_power.c:352:18: note: (near initialization for 'nvec_psy_desc.get_property')
   drivers/staging/nvec/nvec_power.c:363:13: error: invalid storage class for function 'nvec_power_poll'
     363 | static void nvec_power_poll(struct work_struct *work)
         |             ^~~~~~~~~~~~~~~
   drivers/staging/nvec/nvec_power.c:387:12: error: invalid storage class for function 'nvec_power_probe'
     387 | static int nvec_power_probe(struct platform_device *pdev)
         |            ^~~~~~~~~~~~~~~~
   drivers/staging/nvec/nvec_power.c:434:13: error: invalid storage class for function 'nvec_power_remove'
     434 | static void nvec_power_remove(struct platform_device *pdev)
         |             ^~~~~~~~~~~~~~~~~
   drivers/staging/nvec/nvec_power.c:450:11: error: initializer element is not constant
     450 |  .probe = nvec_power_probe,
         |           ^~~~~~~~~~~~~~~~
   drivers/staging/nvec/nvec_power.c:450:11: note: (near initialization for 'nvec_power_driver.probe')
   drivers/staging/nvec/nvec_power.c:451:12: error: initializer element is not constant
     451 |  .remove = nvec_power_remove,
         |            ^~~~~~~~~~~~~~~~~
   drivers/staging/nvec/nvec_power.c:451:12: note: (near initialization for 'nvec_power_driver.remove')
   In file included from include/linux/device.h:32,
                    from include/linux/platform_device.h:13,
                    from drivers/staging/nvec/nvec_power.c:12:
   drivers/staging/nvec/nvec_power.c:457:24: error: invalid storage class for function 'nvec_power_driver_init'
     457 | module_platform_driver(nvec_power_driver);
         |                        ^~~~~~~~~~~~~~~~~
   include/linux/device/driver.h:267:19: note: in definition of macro 'module_driver'
     267 | static int __init __driver##_init(void) \
         |                   ^~~~~~~~
   drivers/staging/nvec/nvec_power.c:457:1: note: in expansion of macro 'module_platform_driver'
     457 | module_platform_driver(nvec_power_driver);
         | ^~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/staging/nvec/nvec_power.c:11:
   include/linux/module.h:132:42: error: invalid storage class for function '__inittest'
     132 |  static inline initcall_t __maybe_unused __inittest(void)  \
         |                                          ^~~~~~~~~~
   include/linux/device/driver.h:271:1: note: in expansion of macro 'module_init'
     271 | module_init(__driver##_init); \
         | ^~~~~~~~~~~
   include/linux/platform_device.h:295:2: note: in expansion of macro 'module_driver'
     295 |  module_driver(__platform_driver, platform_driver_register, \
         |  ^~~~~~~~~~~~~
   drivers/staging/nvec/nvec_power.c:457:1: note: in expansion of macro 'module_platform_driver'
     457 | module_platform_driver(nvec_power_driver);
         | ^~~~~~~~~~~~~~~~~~~~~~
>> drivers/staging/nvec/nvec_power.c:457:1: warning: 'alias' attribute ignored [-Wattributes]
   In file included from include/linux/device.h:32,
                    from include/linux/platform_device.h:13,
                    from drivers/staging/nvec/nvec_power.c:12:
   drivers/staging/nvec/nvec_power.c:457:24: error: invalid storage class for function 'nvec_power_driver_exit'
     457 | module_platform_driver(nvec_power_driver);
         |                        ^~~~~~~~~~~~~~~~~
   include/linux/device/driver.h:272:20: note: in definition of macro 'module_driver'
     272 | static void __exit __driver##_exit(void) \
         |                    ^~~~~~~~
   drivers/staging/nvec/nvec_power.c:457:1: note: in expansion of macro 'module_platform_driver'
     457 | module_platform_driver(nvec_power_driver);
         | ^~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/staging/nvec/nvec_power.c:11:
   include/linux/module.h:140:42: error: invalid storage class for function '__exittest'
     140 |  static inline exitcall_t __maybe_unused __exittest(void)  \
         |                                          ^~~~~~~~~~
   include/linux/device/driver.h:276:1: note: in expansion of macro 'module_exit'
     276 | module_exit(__driver##_exit);
         | ^~~~~~~~~~~
   include/linux/platform_device.h:295:2: note: in expansion of macro 'module_driver'
     295 |  module_driver(__platform_driver, platform_driver_register, \
         |  ^~~~~~~~~~~~~
   drivers/staging/nvec/nvec_power.c:457:1: note: in expansion of macro 'module_platform_driver'
     457 | module_platform_driver(nvec_power_driver);
         | ^~~~~~~~~~~~~~~~~~~~~~
>> drivers/staging/nvec/nvec_power.c:457:1: warning: 'alias' attribute ignored [-Wattributes]
   drivers/staging/nvec/nvec_power.c:462:1: error: expected declaration or statement at end of input
     462 | MODULE_ALIAS("platform:nvec-power");
         | ^~~~~~~~~~~~


vim +/alias +457 drivers/staging/nvec/nvec_power.c

32890b983086136 Marc Dietrich 2011-05-19  456  
9891b1ce6276912 Marc Dietrich 2012-06-24 @457  module_platform_driver(nvec_power_driver);
32890b983086136 Marc Dietrich 2011-05-19  458  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki