[PATCH] fix slab-out-of-bounds Write in betop_probe

[PATCH] fix slab-out-of-bounds Write in betop_probe

[F.A.Sulaiman]

This patch resolves the bug 'KASAN: slab-out-of-bounds Write in betop_probe' reported by Syzbot.
This checkes hid_device's input is non empty before it's been used.

Signed-off-by: F.A. SULAIMAN <asha.16@itfac.mrt.ac.lk>
---
 drivers/hid/hid-betopff.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/hid/hid-betopff.c b/drivers/hid/hid-betopff.c
index 467d789f9bc2..27b57aef9a0a 100644
--- a/drivers/hid/hid-betopff.c
+++ b/drivers/hid/hid-betopff.c
@@ -121,8 +121,18 @@ static int betopff_init(struct hid_device *hid)
 
 static int betop_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
+	struct hid_input *hidinput;
+	struct input_dev *dev;
 	int ret;
 
+	if (list_empty(&hdev->inputs)) {
+		hid_err(hdev, "no inputs found\n");
+		return -ENODEV;
+	}
+
+	hidinput = list_first_entry(&hdev->inputs, struct hid_input, list);
+	dev = hidinput->input;
+
 	if (id->driver_data)
 		hdev->quirks |= HID_QUIRK_MULTI_INPUT;
 
-- 
2.17.1

Re: [PATCH] fix slab-out-of-bounds Write in betop_probe

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2811 bytes --]

Hi "F.A.Sulaiman",

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on hid/for-next]
[also build test WARNING on v5.14-rc6 next-20210816]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/F-A-Sulaiman/fix-slab-out-of-bounds-Write-in-betop_probe/20210817-041818
base:   https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
config: x86_64-randconfig-r023-20210816 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 44d0a99a12ec7ead4d2f5ef649ba05b40f6d463d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/8fc4d7961e182bc2992bee548fa3c33b628ffadd
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review F-A-Sulaiman/fix-slab-out-of-bounds-Write-in-betop_probe/20210817-041818
        git checkout 8fc4d7961e182bc2992bee548fa3c33b628ffadd
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/hid/hid-betopff.c:118:20: warning: variable 'dev' set but not used [-Wunused-but-set-variable]
           struct input_dev *dev;
                             ^
   1 warning generated.


vim +/dev +118 drivers/hid/hid-betopff.c

   114	
   115	static int betop_probe(struct hid_device *hdev, const struct hid_device_id *id)
   116	{
   117		struct hid_input *hidinput;
 > 118		struct input_dev *dev;
   119		int ret;
   120	
   121		if (list_empty(&hdev->inputs)) {
   122			hid_err(hdev, "no inputs found\n");
   123			return -ENODEV;
   124		}
   125	
   126		hidinput = list_first_entry(&hdev->inputs, struct hid_input, list);
   127		dev = hidinput->input;
   128	
   129		if (id->driver_data)
   130			hdev->quirks |= HID_QUIRK_MULTI_INPUT;
   131	
   132		ret = hid_parse(hdev);
   133		if (ret) {
   134			hid_err(hdev, "parse failed\n");
   135			goto err;
   136		}
   137	
   138		ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
   139		if (ret) {
   140			hid_err(hdev, "hw start failed\n");
   141			goto err;
   142		}
   143	
   144		betopff_init(hdev);
   145	
   146		return 0;
   147	err:
   148		return ret;
   149	}
   150	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34480 bytes --]

Re: [PATCH] fix slab-out-of-bounds Write in betop_probe

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2571 bytes --]

Hi "F.A.Sulaiman",

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on hid/for-next]
[also build test WARNING on v5.14-rc6 next-20210816]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/F-A-Sulaiman/fix-slab-out-of-bounds-Write-in-betop_probe/20210817-041818
base:   https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/8fc4d7961e182bc2992bee548fa3c33b628ffadd
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review F-A-Sulaiman/fix-slab-out-of-bounds-Write-in-betop_probe/20210817-041818
        git checkout 8fc4d7961e182bc2992bee548fa3c33b628ffadd
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/hid/hid-betopff.c: In function 'betop_probe':
>> drivers/hid/hid-betopff.c:118:20: warning: variable 'dev' set but not used [-Wunused-but-set-variable]
     118 |  struct input_dev *dev;
         |                    ^~~


vim +/dev +118 drivers/hid/hid-betopff.c

   114	
   115	static int betop_probe(struct hid_device *hdev, const struct hid_device_id *id)
   116	{
   117		struct hid_input *hidinput;
 > 118		struct input_dev *dev;
   119		int ret;
   120	
   121		if (list_empty(&hdev->inputs)) {
   122			hid_err(hdev, "no inputs found\n");
   123			return -ENODEV;
   124		}
   125	
   126		hidinput = list_first_entry(&hdev->inputs, struct hid_input, list);
   127		dev = hidinput->input;
   128	
   129		if (id->driver_data)
   130			hdev->quirks |= HID_QUIRK_MULTI_INPUT;
   131	
   132		ret = hid_parse(hdev);
   133		if (ret) {
   134			hid_err(hdev, "parse failed\n");
   135			goto err;
   136		}
   137	
   138		ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
   139		if (ret) {
   140			hid_err(hdev, "hw start failed\n");
   141			goto err;
   142		}
   143	
   144		betopff_init(hdev);
   145	
   146		return 0;
   147	err:
   148		return ret;
   149	}
   150	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 64790 bytes --]

Re: [PATCH] fix slab-out-of-bounds Write in betop_probe

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 14680 bytes --]

Hi "F.A.Sulaiman",

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on hid/for-next]
[also build test WARNING on v5.14-rc6 next-20210817]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/F-A-Sulaiman/fix-slab-out-of-bounds-Write-in-betop_probe/20210817-041818
base:   https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
config: x86_64-randconfig-c007-20210816 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 44d0a99a12ec7ead4d2f5ef649ba05b40f6d463d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/8fc4d7961e182bc2992bee548fa3c33b628ffadd
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review F-A-Sulaiman/fix-slab-out-of-bounds-Write-in-betop_probe/20210817-041818
        git checkout 8fc4d7961e182bc2992bee548fa3c33b628ffadd
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 clang-analyzer 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


clang-analyzer warnings: (new ones prefixed by >>)
   set_temp_reg(HYST, temp_hyst);
   ^
   drivers/hwmon/asb100.c:443:19: note: expanded from macro 'set_temp_reg'
                   data->reg[nr] = LM75_TEMP_TO_REG(val); \
                                   ^~~~~~~~~~~~~~~~~~~~~
   drivers/hwmon/lm75.h:27:14: note: Assuming '__UNIQUE_ID___x271' is <= '__UNIQUE_ID___y272'
           int ntemp = clamp_val(temp, LM75_TEMP_MIN, LM75_TEMP_MAX);
                       ^
   include/linux/minmax.h:137:32: note: expanded from macro 'clamp_val'
   #define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi)
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:124:48: note: expanded from macro 'clamp_t'
   #define clamp_t(type, val, lo, hi) min_t(type, max_t(type, val, lo), hi)
                                      ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:112:27: note: expanded from macro 'max_t'
   #define max_t(type, x, y)       __careful_cmp((type)(x), (type)(y), >)
                                   ^
   note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/minmax.h:104:48: note: expanded from macro 'min_t'
   #define min_t(type, x, y)       __careful_cmp((type)(x), (type)(y), <)
                                   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:38:14: note: expanded from macro '__careful_cmp'
                   __cmp_once(x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y), op))
                   ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:31:25: note: expanded from macro '__cmp_once'
                   typeof(x) unique_x = (x);               \
                                         ^
   drivers/hwmon/lm75.h:27:14: note: '?' condition is false
           int ntemp = clamp_val(temp, LM75_TEMP_MIN, LM75_TEMP_MAX);
                       ^
   include/linux/minmax.h:137:32: note: expanded from macro 'clamp_val'
   #define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi)
                                  ^
   include/linux/minmax.h:124:48: note: expanded from macro 'clamp_t'
   #define clamp_t(type, val, lo, hi) min_t(type, max_t(type, val, lo), hi)
                                                  ^
   include/linux/minmax.h:112:27: note: expanded from macro 'max_t'
   #define max_t(type, x, y)       __careful_cmp((type)(x), (type)(y), >)
                                   ^
   include/linux/minmax.h:38:3: note: expanded from macro '__careful_cmp'
                   __cmp_once(x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y), op))
                   ^
   include/linux/minmax.h:33:3: note: expanded from macro '__cmp_once'
                   __cmp(unique_x, unique_y, op); })
                   ^
   include/linux/minmax.h:28:26: note: expanded from macro '__cmp'
   #define __cmp(x, y, op) ((x) op (y) ? (x) : (y))
                            ^
   drivers/hwmon/lm75.h:27:14: note: '__UNIQUE_ID___x273' is < '__UNIQUE_ID___y274'
           int ntemp = clamp_val(temp, LM75_TEMP_MIN, LM75_TEMP_MAX);
                       ^
   include/linux/minmax.h:137:32: note: expanded from macro 'clamp_val'
   #define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi)
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:124:36: note: expanded from macro 'clamp_t'
   #define clamp_t(type, val, lo, hi) min_t(type, max_t(type, val, lo), hi)
                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:104:27: note: expanded from macro 'min_t'
   #define min_t(type, x, y)       __careful_cmp((type)(x), (type)(y), <)
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:38:3: note: expanded from macro '__careful_cmp'
                   __cmp_once(x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y), op))
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:33:3: note: expanded from macro '__cmp_once'
                   __cmp(unique_x, unique_y, op); })
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:28:26: note: expanded from macro '__cmp'
   #define __cmp(x, y, op) ((x) op (y) ? (x) : (y))
                            ^~~
   drivers/hwmon/lm75.h:27:14: note: '?' condition is true
           int ntemp = clamp_val(temp, LM75_TEMP_MIN, LM75_TEMP_MAX);
                       ^
   include/linux/minmax.h:137:32: note: expanded from macro 'clamp_val'
   #define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi)
                                  ^
   include/linux/minmax.h:124:36: note: expanded from macro 'clamp_t'
   #define clamp_t(type, val, lo, hi) min_t(type, max_t(type, val, lo), hi)
                                      ^
   include/linux/minmax.h:104:27: note: expanded from macro 'min_t'
   #define min_t(type, x, y)       __careful_cmp((type)(x), (type)(y), <)
                                   ^
   include/linux/minmax.h:38:3: note: expanded from macro '__careful_cmp'
                   __cmp_once(x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y), op))
                   ^
   include/linux/minmax.h:33:3: note: expanded from macro '__cmp_once'
                   __cmp(unique_x, unique_y, op); })
                   ^
   include/linux/minmax.h:28:26: note: expanded from macro '__cmp'
   #define __cmp(x, y, op) ((x) op (y) ? (x) : (y))
                            ^
   drivers/hwmon/lm75.h:29:12: note: 'ntemp' is < 0
           ntemp += (ntemp < 0 ? -250 : 250);
                     ^~~~~
   drivers/hwmon/lm75.h:29:12: note: '?' condition is true
   drivers/hwmon/lm75.h:30:29: note: The result of the left shift is undefined because the left operand is negative
           return (u16)((ntemp / 500) << 7);
                        ~~~~~~~~~~~~~ ^
   Suppressed 3 warnings (3 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   5 warnings generated.
>> drivers/hid/hid-betopff.c:127:2: warning: Value stored to 'dev' is never read [clang-analyzer-deadcode.DeadStores]
           dev = hidinput->input;
           ^     ~~~~~~~~~~~~~~~
   drivers/hid/hid-betopff.c:127:2: note: Value stored to 'dev' is never read
           dev = hidinput->input;
           ^     ~~~~~~~~~~~~~~~
   Suppressed 4 warnings (3 in non-user code, 1 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   3 warnings generated.
   Suppressed 3 warnings (3 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   4 warnings generated.
   include/linux/hid.h:1007:9: warning: Access to field 'name' results in a dereference of a null pointer (loaded from variable 'input') [clang-analyzer-core.NullDereference]
                                       input->name, c, type);
                                       ^
   drivers/hid/hid-corsair.c:630:6: note: Assuming the condition is false
           if ((usage->hid & HID_USAGE_PAGE) != HID_UP_KEYBOARD)
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/hid/hid-corsair.c:630:2: note: Taking false branch
           if ((usage->hid & HID_USAGE_PAGE) != HID_UP_KEYBOARD)
           ^
   drivers/hid/hid-corsair.c:634:6: note: 'gkey' is not equal to 0
           if (gkey != 0) {
               ^~~~
   drivers/hid/hid-corsair.c:634:2: note: Taking true branch
           if (gkey != 0) {
           ^
   drivers/hid/hid-corsair.c:635:3: note: Calling 'hid_map_usage_clear'
                   hid_map_usage_clear(input, usage, bit, max, EV_KEY,
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/hid.h:1035:2: note: Calling 'hid_map_usage'
           hid_map_usage(hidinput, usage, bit, max, type, c);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/hid.h:982:2: note: 'input' initialized here
           struct input_dev *input = hidinput->input;
           ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/hid.h:986:2: note: Control jumps to 'case 1:'  at line 995
           switch (type) {
           ^
   include/linux/hid.h:998:3: note:  Execution continues on line 1005
                   break;
                   ^
   include/linux/hid.h:1005:15: note: Assuming 'c' is <= 'limit'
           if (unlikely(c > limit || !bmap)) {
                        ^
   include/linux/compiler.h:78:42: note: expanded from macro 'unlikely'
   # define unlikely(x)    __builtin_expect(!!(x), 0)
                                               ^
   include/linux/hid.h:1005:15: note: Left side of '||' is false
           if (unlikely(c > limit || !bmap)) {
                        ^
   include/linux/hid.h:1005:28: note: Assuming 'bmap' is null
           if (unlikely(c > limit || !bmap)) {
                                     ^
   include/linux/compiler.h:78:42: note: expanded from macro 'unlikely'
   # define unlikely(x)    __builtin_expect(!!(x), 0)
                                               ^
   include/linux/hid.h:1005:28: note: Assuming pointer value is null
           if (unlikely(c > limit || !bmap)) {
                                     ^
   include/linux/compiler.h:78:42: note: expanded from macro 'unlikely'
   # define unlikely(x)    __builtin_expect(!!(x), 0)
                                               ^
   include/linux/hid.h:1005:2: note: Taking true branch
           if (unlikely(c > limit || !bmap)) {
           ^
   include/linux/hid.h:1006:3: note: Assuming the condition is true
                   pr_warn_ratelimited("%s: Invalid code %d type %d\n",
                   ^
   include/linux/printk.h:574:2: note: expanded from macro 'pr_warn_ratelimited'
           printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/printk.h:557:6: note: expanded from macro 'printk_ratelimited'
           if (__ratelimit(&_rs))                                          \
               ^~~~~~~~~~~~~~~~~
   include/linux/ratelimit_types.h:41:28: note: expanded from macro '__ratelimit'
   #define __ratelimit(state) ___ratelimit(state, __func__)
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/hid.h:1006:3: note: Taking true branch
                   pr_warn_ratelimited("%s: Invalid code %d type %d\n",
                   ^
   include/linux/printk.h:574:2: note: expanded from macro 'pr_warn_ratelimited'
           printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
           ^
   include/linux/printk.h:557:2: note: expanded from macro 'printk_ratelimited'
           if (__ratelimit(&_rs))                                          \
           ^
   include/linux/hid.h:1007:9: note: Access to field 'name' results in a dereference of a null pointer (loaded from variable 'input')
                                       input->name, c, type);
                                       ^
   include/linux/printk.h:574:49: note: expanded from macro 'pr_warn_ratelimited'
           printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
                                                          ^~~~~~~~~~~
   include/linux/printk.h:558:17: note: expanded from macro 'printk_ratelimited'
                   printk(fmt, ##__VA_ARGS__);                             \
                                 ^~~~~~~~~~~
   Suppressed 3 warnings (3 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   4 warnings generated.
   kernel/static_call.c:275:47: warning: Access to field 'func' results in a dereference of a null pointer (loaded from variable 'key') [clang-analyzer-core.NullDereference]
                   arch_static_call_transform(site_addr, NULL, key->func,

vim +/dev +127 drivers/hid/hid-betopff.c

   114	
   115	static int betop_probe(struct hid_device *hdev, const struct hid_device_id *id)
   116	{
   117		struct hid_input *hidinput;
   118		struct input_dev *dev;
   119		int ret;
   120	
   121		if (list_empty(&hdev->inputs)) {
   122			hid_err(hdev, "no inputs found\n");
   123			return -ENODEV;
   124		}
   125	
   126		hidinput = list_first_entry(&hdev->inputs, struct hid_input, list);
 > 127		dev = hidinput->input;
   128	
   129		if (id->driver_data)
   130			hdev->quirks |= HID_QUIRK_MULTI_INPUT;
   131	
   132		ret = hid_parse(hdev);
   133		if (ret) {
   134			hid_err(hdev, "parse failed\n");
   135			goto err;
   136		}
   137	
   138		ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
   139		if (ret) {
   140			hid_err(hdev, "hw start failed\n");
   141			goto err;
   142		}
   143	
   144		betopff_init(hdev);
   145	
   146		return 0;
   147	err:
   148		return ret;
   149	}
   150	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35425 bytes --]

Re: [PATCH] fix slab-out-of-bounds Write in betop_probe

[Jiri Kosina]

On Tue, 17 Aug 2021, F.A.Sulaiman wrote:

> This patch resolves the bug 'KASAN: slab-out-of-bounds Write in betop_probe' reported by Syzbot.
> This checkes hid_device's input is non empty before it's been used.
> 
> Signed-off-by: F.A. SULAIMAN <asha.16@itfac.mrt.ac.lk>
> ---
>  drivers/hid/hid-betopff.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/hid/hid-betopff.c b/drivers/hid/hid-betopff.c
> index 467d789f9bc2..27b57aef9a0a 100644
> --- a/drivers/hid/hid-betopff.c
> +++ b/drivers/hid/hid-betopff.c
> @@ -121,8 +121,18 @@ static int betopff_init(struct hid_device *hid)
>  
>  static int betop_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  {
> +	struct hid_input *hidinput;
> +	struct input_dev *dev;
>  	int ret;
>  
> +	if (list_empty(&hdev->inputs)) {
> +		hid_err(hdev, "no inputs found\n");
> +		return -ENODEV;
> +	}
> +
> +	hidinput = list_first_entry(&hdev->inputs, struct hid_input, list);
> +	dev = hidinput->input;
> +

Thanks for the fix. Syzbot is right though -- what is the point of the 
above assignment? Could you please resubmit a fixed up version?

Thanks,

-- 
Jiri Kosina
SUSE Labs

[PATCH v2] HID: betop: fix slab-out-of-bounds Write in betop_probe

[F.A.Sulaiman]

Syzbot reported slab-out-of-bounds Write bug in hid-betopff driver.
The problem is the driver assumes the device must have an input report but
some malicious devices violate this assumption.

So this patch checks hid_device's input is non empty before it's been used.

Reported-by: syzbot+07efed3bc5a1407bd742@syzkaller.appspotmail.com
Signed-off-by: F.A. SULAIMAN <asha.16@itfac.mrt.ac.lk>
---
 drivers/hid/hid-betopff.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hid/hid-betopff.c b/drivers/hid/hid-betopff.c
index 0790fbd3fc9a..2d62bde21413 100644
--- a/drivers/hid/hid-betopff.c
+++ b/drivers/hid/hid-betopff.c
@@ -116,6 +116,11 @@ static int betop_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	int ret;
 
+	if (list_empty(&hdev->inputs)) {
+		hid_err(hdev, "no inputs found\n");
+		return -ENODEV;
+	}
+
 	if (id->driver_data)
 		hdev->quirks |= HID_QUIRK_MULTI_INPUT;
 
-- 
2.17.1

Re: [PATCH v2] HID: betop: fix slab-out-of-bounds Write in betop_probe

[Pavel Skripkin]

On 8/22/21 4:43 PM, F.A.Sulaiman wrote:
> Syzbot reported slab-out-of-bounds Write bug in hid-betopff driver.
> The problem is the driver assumes the device must have an input report but
> some malicious devices violate this assumption.
> 
> So this patch checks hid_device's input is non empty before it's been used.
> 
> Reported-by: syzbot+07efed3bc5a1407bd742@syzkaller.appspotmail.com
> Signed-off-by: F.A. SULAIMAN <asha.16@itfac.mrt.ac.lk>
> ---
>   drivers/hid/hid-betopff.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/hid/hid-betopff.c b/drivers/hid/hid-betopff.c
> index 0790fbd3fc9a..2d62bde21413 100644
> --- a/drivers/hid/hid-betopff.c
> +++ b/drivers/hid/hid-betopff.c
> @@ -116,6 +116,11 @@ static int betop_probe(struct hid_device *hdev, const struct hid_device_id *id)
>   {
>   	int ret;
>   
> +	if (list_empty(&hdev->inputs)) {
> +		hid_err(hdev, "no inputs found\n");
> +		return -ENODEV;
> +	}
> +
>   	if (id->driver_data)
>   		hdev->quirks |= HID_QUIRK_MULTI_INPUT;
>   
> 

I am still able to trigger reported slab-out-bound with this patch 
applied, please move this sanity check inside betopff_init().


Jiri, does it make sense to add proper error handling of betopff_init()? 
Nowadays betop_probe() just ignores betopff_init() return value. It 
looks wrong to me.


I think, Asha can prepare a patch series with these 2 changes



With regards,
Pavel Skripkin

[PATCH v3] HID: betop: fix slab-out-of-bounds Write in betop_probe

[F.A.Sulaiman]

Syzbot reported slab-out-of-bounds Write bug in hid-betopff driver.
The problem is the driver assumes the device must have an input report but
some malicious devices violate this assumption.

So this patch checks hid_device's input is non empty before it's been used.

Reported-by: syzbot+07efed3bc5a1407bd742@syzkaller.appspotmail.com
Signed-off-by: F.A. SULAIMAN <asha.16@itfac.mrt.ac.lk>
---
 drivers/hid/hid-betopff.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-betopff.c b/drivers/hid/hid-betopff.c
index 0790fbd3fc9a..467d789f9bc2 100644
--- a/drivers/hid/hid-betopff.c
+++ b/drivers/hid/hid-betopff.c
@@ -56,15 +56,22 @@ static int betopff_init(struct hid_device *hid)
 {
 	struct betopff_device *betopff;
 	struct hid_report *report;
-	struct hid_input *hidinput =
-			list_first_entry(&hid->inputs, struct hid_input, list);
+	struct hid_input *hidinput;
 	struct list_head *report_list =
 			&hid->report_enum[HID_OUTPUT_REPORT].report_list;
-	struct input_dev *dev = hidinput->input;
+	struct input_dev *dev;
 	int field_count = 0;
 	int error;
 	int i, j;
 
+	if (list_empty(&hid->inputs)) {
+		hid_err(hid, "no inputs found\n");
+		return -ENODEV;
+	}
+
+	hidinput = list_first_entry(&hid->inputs, struct hid_input, list);
+	dev = hidinput->input;
+
 	if (list_empty(report_list)) {
 		hid_err(hid, "no output reports found\n");
 		return -ENODEV;
-- 
2.17.1

Re: [PATCH v3] HID: betop: fix slab-out-of-bounds Write in betop_probe

[Pavel Skripkin]

On 8/24/21 6:07 PM, F.A.Sulaiman wrote:
> Syzbot reported slab-out-of-bounds Write bug in hid-betopff driver.
> The problem is the driver assumes the device must have an input report but
> some malicious devices violate this assumption.
> 
> So this patch checks hid_device's input is non empty before it's been used.
> 
> Reported-by: syzbot+07efed3bc5a1407bd742@syzkaller.appspotmail.com
> Signed-off-by: F.A. SULAIMAN <asha.16@itfac.mrt.ac.lk>


Reviewed-by: Pavel Skripkin <paskripkin@gmail.com>


> ---
>   drivers/hid/hid-betopff.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hid/hid-betopff.c b/drivers/hid/hid-betopff.c
> index 0790fbd3fc9a..467d789f9bc2 100644
> --- a/drivers/hid/hid-betopff.c
> +++ b/drivers/hid/hid-betopff.c
> @@ -56,15 +56,22 @@ static int betopff_init(struct hid_device *hid)
>   {
>   	struct betopff_device *betopff;
>   	struct hid_report *report;
> -	struct hid_input *hidinput =
> -			list_first_entry(&hid->inputs, struct hid_input, list);
> +	struct hid_input *hidinput;
>   	struct list_head *report_list =
>   			&hid->report_enum[HID_OUTPUT_REPORT].report_list;
> -	struct input_dev *dev = hidinput->input;
> +	struct input_dev *dev;
>   	int field_count = 0;
>   	int error;
>   	int i, j;
>   
> +	if (list_empty(&hid->inputs)) {
> +		hid_err(hid, "no inputs found\n");
> +		return -ENODEV;
> +	}
> +
> +	hidinput = list_first_entry(&hid->inputs, struct hid_input, list);
> +	dev = hidinput->input;
> +
>   	if (list_empty(report_list)) {
>   		hid_err(hid, "no output reports found\n");
>   		return -ENODEV;
> 

Re: [PATCH v3] HID: betop: fix slab-out-of-bounds Write in betop_probe

[Jiri Kosina]

On Tue, 24 Aug 2021, F.A.Sulaiman wrote:

> Syzbot reported slab-out-of-bounds Write bug in hid-betopff driver.
> The problem is the driver assumes the device must have an input report but
> some malicious devices violate this assumption.
> 
> So this patch checks hid_device's input is non empty before it's been used.
> 
> Reported-by: syzbot+07efed3bc5a1407bd742@syzkaller.appspotmail.com
> Signed-off-by: F.A. SULAIMAN <asha.16@itfac.mrt.ac.lk>

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs