media: cx25821: prevent out-of-bounds read on array card
diff mbox

Message ID 20180131173309.16377-1-colin.king@canonical.com
State New
Headers show

Commit Message

Colin King Jan. 31, 2018, 5:33 p.m. UTC
From: Colin Ian King <colin.king@canonical.com>

Currently an out of range dev->nr is detected by just reporting the
issue and later on an out-of-bounds read on array card occurs because
of this. Fix this by checking the upper range of dev->nr with the size
of array card (removes the hard coded size), move this check earlier
and also exit with the error -ENOSYS to avoid the later out-of-bounds
array read.

Detected by CoverityScan, CID#711191 ("Out-of-bounds-read")

Fixes: commit 02b20b0b4cde ("V4L/DVB (12730): Add conexant cx25821 driver")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/media/pci/cx25821/cx25821-core.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

kbuild test robot Feb. 3, 2018, 6:41 a.m. UTC | #1
Hi Colin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.15 next-20180202]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Colin-King/media-cx25821-prevent-out-of-bounds-read-on-array-card/20180203-130958
base:   git://linuxtv.org/media_tree.git master
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

All warnings (new ones prefixed by >>):

   In file included from include/linux/printk.h:7:0,
                    from include/linux/kernel.h:14,
                    from include/linux/list.h:9,
                    from include/linux/kobject.h:20,
                    from include/linux/device.h:17,
                    from include/linux/i2c.h:30,
                    from drivers/media/pci/cx25821/cx25821-core.c:22:
   drivers/media/pci/cx25821/cx25821-core.c: In function 'cx25821_dev_setup':
>> include/linux/kern_levels.h:5:18: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'unsigned int' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/kern_levels.h:14:19: note: in expansion of macro 'KERN_SOH'
    #define KERN_INFO KERN_SOH "6" /* informational */
                      ^~~~~~~~
   include/linux/printk.h:308:9: note: in expansion of macro 'KERN_INFO'
     printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
            ^~~~~~~~~
>> drivers/media/pci/cx25821/cx25821.h:380:2: note: in expansion of macro 'pr_info'
     pr_info("(%d): " fmt, dev->board, ##args)
     ^~~~~~~
>> drivers/media/pci/cx25821/cx25821-core.c:871:3: note: in expansion of macro 'CX25821_INFO'
      CX25821_INFO("dev->nr >= %ld", ARRAY_SIZE(card));
      ^~~~~~~~~~~~

vim +/pr_info +380 drivers/media/pci/cx25821/cx25821.h

02b20b0b drivers/staging/cx25821/cx25821.h Mauro Carvalho Chehab 2009-09-15  374  
36d89f7d drivers/staging/cx25821/cx25821.h Joe Perches           2010-11-07  375  #define CX25821_ERR(fmt, args...)			\
36d89f7d drivers/staging/cx25821/cx25821.h Joe Perches           2010-11-07  376  	pr_err("(%d): " fmt, dev->board, ##args)
36d89f7d drivers/staging/cx25821/cx25821.h Joe Perches           2010-11-07  377  #define CX25821_WARN(fmt, args...)			\
36d89f7d drivers/staging/cx25821/cx25821.h Joe Perches           2010-11-07  378  	pr_warn("(%d): " fmt, dev->board, ##args)
36d89f7d drivers/staging/cx25821/cx25821.h Joe Perches           2010-11-07  379  #define CX25821_INFO(fmt, args...)			\
36d89f7d drivers/staging/cx25821/cx25821.h Joe Perches           2010-11-07 @380  	pr_info("(%d): " fmt, dev->board, ##args)
02b20b0b drivers/staging/cx25821/cx25821.h Mauro Carvalho Chehab 2009-09-15  381  

:::::: The code at line 380 was first introduced by commit
:::::: 36d89f7de4a4937848de86d9b35cb03a9f0357e1 [media] drivers/staging/cx25821: Use pr_fmt and pr_<level>

:::::: TO: Joe Perches <joe@perches.com>
:::::: CC: Mauro Carvalho Chehab <mchehab@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kbuild test robot Feb. 3, 2018, 9:14 a.m. UTC | #2
Hi Colin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.15 next-20180202]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Colin-King/media-cx25821-prevent-out-of-bounds-read-on-array-card/20180203-130958
base:   git://linuxtv.org/media_tree.git master
config: i386-randconfig-sb0-02031534 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:15:0,
                    from include/linux/list.h:9,
                    from include/linux/kobject.h:20,
                    from include/linux/device.h:17,
                    from include/linux/i2c.h:30,
                    from drivers/media/pci/cx25821/cx25821-core.c:22:
   drivers/media/pci/cx25821/cx25821-core.c: In function 'cx25821_dev_setup':
>> include/linux/build_bug.h:30:45: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'unsigned int' [-Wformat=]
    #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); }))
                                                ^
   include/linux/compiler-gcc.h:65:28: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
    #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
                               ^
   include/linux/kernel.h:71:59: note: in expansion of macro '__must_be_array'
    #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
                                                              ^
>> include/linux/printk.h:308:34: note: in expansion of macro 'ARRAY_SIZE'
     printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
                                     ^
   drivers/media/pci/cx25821/cx25821.h:380:2: note: in expansion of macro 'pr_info'
     pr_info("(%d): " fmt, dev->board, ##args)
     ^
   drivers/media/pci/cx25821/cx25821-core.c:871:3: note: in expansion of macro 'CX25821_INFO'
      CX25821_INFO("dev->nr >= %ld", ARRAY_SIZE(card));
      ^
--
   In file included from include/linux/kernel.h:15:0,
                    from include/linux/list.h:9,
                    from include/linux/kobject.h:20,
                    from include/linux/device.h:17,
                    from include/linux/i2c.h:30,
                    from drivers/media//pci/cx25821/cx25821-core.c:22:
   drivers/media//pci/cx25821/cx25821-core.c: In function 'cx25821_dev_setup':
>> include/linux/build_bug.h:30:45: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'unsigned int' [-Wformat=]
    #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); }))
                                                ^
   include/linux/compiler-gcc.h:65:28: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
    #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
                               ^
   include/linux/kernel.h:71:59: note: in expansion of macro '__must_be_array'
    #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
                                                              ^
>> include/linux/printk.h:308:34: note: in expansion of macro 'ARRAY_SIZE'
     printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
                                     ^
   drivers/media//pci/cx25821/cx25821.h:380:2: note: in expansion of macro 'pr_info'
     pr_info("(%d): " fmt, dev->board, ##args)
     ^
   drivers/media//pci/cx25821/cx25821-core.c:871:3: note: in expansion of macro 'CX25821_INFO'
      CX25821_INFO("dev->nr >= %ld", ARRAY_SIZE(card));
      ^

vim +/ARRAY_SIZE +308 include/linux/printk.h

968ab183 Linus Torvalds 2010-11-15  287  
6e099f55 Dan Streetman  2014-06-04  288  /*
6e099f55 Dan Streetman  2014-06-04  289   * These can be used to print at the various log levels.
6e099f55 Dan Streetman  2014-06-04  290   * All of these will print unconditionally, although note that pr_debug()
6e099f55 Dan Streetman  2014-06-04  291   * and other debug macros are compiled out unless either DEBUG is defined
6e099f55 Dan Streetman  2014-06-04  292   * or CONFIG_DYNAMIC_DEBUG is set.
6e099f55 Dan Streetman  2014-06-04  293   */
a0cba217 Linus Torvalds 2016-08-09  294  #define pr_emerg(fmt, ...) \
a0cba217 Linus Torvalds 2016-08-09  295  	printk(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
a0cba217 Linus Torvalds 2016-08-09  296  #define pr_alert(fmt, ...) \
a0cba217 Linus Torvalds 2016-08-09  297  	printk(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
a0cba217 Linus Torvalds 2016-08-09  298  #define pr_crit(fmt, ...) \
a0cba217 Linus Torvalds 2016-08-09  299  	printk(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
a0cba217 Linus Torvalds 2016-08-09  300  #define pr_err(fmt, ...) \
a0cba217 Linus Torvalds 2016-08-09  301  	printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
a0cba217 Linus Torvalds 2016-08-09  302  #define pr_warning(fmt, ...) \
a0cba217 Linus Torvalds 2016-08-09  303  	printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
a0cba217 Linus Torvalds 2016-08-09  304  #define pr_warn pr_warning
a0cba217 Linus Torvalds 2016-08-09  305  #define pr_notice(fmt, ...) \
a0cba217 Linus Torvalds 2016-08-09  306  	printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
a0cba217 Linus Torvalds 2016-08-09  307  #define pr_info(fmt, ...) \
a0cba217 Linus Torvalds 2016-08-09 @308  	printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
7b1460ec Steven Rostedt 2015-04-15  309  /*
7b1460ec Steven Rostedt 2015-04-15  310   * Like KERN_CONT, pr_cont() should only be used when continuing
7b1460ec Steven Rostedt 2015-04-15  311   * a line with no newline ('\n') enclosed. Otherwise it defaults
7b1460ec Steven Rostedt 2015-04-15  312   * back to KERN_DEFAULT.
7b1460ec Steven Rostedt 2015-04-15  313   */
968ab183 Linus Torvalds 2010-11-15  314  #define pr_cont(fmt, ...) \
968ab183 Linus Torvalds 2010-11-15  315  	printk(KERN_CONT fmt, ##__VA_ARGS__)
968ab183 Linus Torvalds 2010-11-15  316  

:::::: The code at line 308 was first introduced by commit
:::::: a0cba2179ea4c1820fce2ee046b6ed90ecc56196 Revert "printk: create pr_<level> functions"

:::::: TO: Linus Torvalds <torvalds@linux-foundation.org>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Patch
diff mbox

diff --git a/drivers/media/pci/cx25821/cx25821-core.c b/drivers/media/pci/cx25821/cx25821-core.c
index 04aa4a68a0ae..c1184e73cb2d 100644
--- a/drivers/media/pci/cx25821/cx25821-core.c
+++ b/drivers/media/pci/cx25821/cx25821-core.c
@@ -867,6 +867,10 @@  static int cx25821_dev_setup(struct cx25821_dev *dev)
 	dev->nr = ++cx25821_devcount;
 	sprintf(dev->name, "cx25821[%d]", dev->nr);
 
+	if (dev->nr >= ARRAY_SIZE(card)) {
+		CX25821_INFO("dev->nr >= %ld", ARRAY_SIZE(card));
+		return -ENODEV;
+	}
 	if (dev->pci->device != 0x8210) {
 		pr_info("%s(): Exiting. Incorrect Hardware device = 0x%02x\n",
 			__func__, dev->pci->device);
@@ -882,9 +886,6 @@  static int cx25821_dev_setup(struct cx25821_dev *dev)
 		dev->channels[i].sram_channels = &cx25821_sram_channels[i];
 	}
 
-	if (dev->nr > 1)
-		CX25821_INFO("dev->nr > 1!");
-
 	/* board config */
 	dev->board = 1;		/* card[dev->nr]; */
 	dev->_max_num_decoders = MAX_DECODERS;