linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ASoC: wm_adsp: Don't overrun firmware file buffer when reading region data
@ 2016-12-19 15:18 Richard Fitzgerald
  2016-12-19 19:16 ` kbuild test robot
  2016-12-19 19:20 ` kbuild test robot
  0 siblings, 2 replies; 3+ messages in thread
From: Richard Fitzgerald @ 2016-12-19 15:18 UTC (permalink / raw)
  To: broonie; +Cc: linux-kernel, alsa-devel, patches

Protect against corrupt firmware files by ensuring that the length we
get for the data in a region actually lies within the available firmware
file data buffer.

Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 593b7d1..91ccf62 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -1551,7 +1551,7 @@ static int wm_adsp_load(struct wm_adsp *dsp)
 	const struct wmfw_region *region;
 	const struct wm_adsp_region *mem;
 	const char *region_name;
-	char *file, *text;
+	char *file, *text = NULL;
 	struct wm_adsp_buf *buf;
 	unsigned int reg;
 	int regions = 0;
@@ -1700,10 +1700,21 @@ static int wm_adsp_load(struct wm_adsp *dsp)
 			 regions, le32_to_cpu(region->len), offset,
 			 region_name);
 
+		if ((pos + le32_to_cpu(region->len) + sizeof(*region)) >
+		    firmware->size) {
+			adsp_err(dsp,
+				 "%s.%d: %s region len %d bytes exceeds file length %d\n",
+				 file, regions, region_name,
+				 le32_to_cpu(region->len), firmware->size);
+			ret = -EINVAL;
+			goto out_fw;
+		}
+
 		if (text) {
 			memcpy(text, region->data, le32_to_cpu(region->len));
 			adsp_info(dsp, "%s: %s\n", file, text);
 			kfree(text);
+			text = NULL;
 		}
 
 		if (reg) {
@@ -1748,6 +1759,7 @@ static int wm_adsp_load(struct wm_adsp *dsp)
 	regmap_async_complete(regmap);
 	wm_adsp_buf_free(&buf_list);
 	release_firmware(firmware);
+	kfree(text);
 out:
 	kfree(file);
 
@@ -2233,6 +2245,17 @@ static int wm_adsp_load_coeff(struct wm_adsp *dsp)
 		}
 
 		if (reg) {
+			if ((pos + le32_to_cpu(blk->len) + sizeof(*blk)) >
+			    firmware->size) {
+				adsp_err(dsp,
+					 "%s.%d: %s region len %d bytes exceeds file length %d\n",
+					 file, blocks, region_name,
+					 le32_to_cpu(blk->len),
+					 firmware->size);
+				ret = -EINVAL;
+				goto out_fw;
+			}
+
 			buf = wm_adsp_buf_alloc(blk->data,
 						le32_to_cpu(blk->len),
 						&buf_list);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] ASoC: wm_adsp: Don't overrun firmware file buffer when reading region data
  2016-12-19 15:18 [PATCH] ASoC: wm_adsp: Don't overrun firmware file buffer when reading region data Richard Fitzgerald
@ 2016-12-19 19:16 ` kbuild test robot
  2016-12-19 19:20 ` kbuild test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2016-12-19 19:16 UTC (permalink / raw)
  To: Richard Fitzgerald; +Cc: kbuild-all, broonie, linux-kernel, alsa-devel, patches

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

Hi Richard,

[auto build test WARNING on asoc/for-next]
[also build test WARNING on v4.9 next-20161219]
[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/Richard-Fitzgerald/ASoC-wm_adsp-Don-t-overrun-firmware-file-buffer-when-reading-region-data/20161220-021733
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: tile-allmodconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 4.6.2
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=tile 

All warnings (new ones prefixed by >>):

   sound/soc/codecs/wm_adsp.c: In function 'wm_adsp_load':
>> sound/soc/codecs/wm_adsp.c:1705:4: warning: format '%d' expects argument of type 'int', but argument 8 has type 'size_t' [-Wformat]
   sound/soc/codecs/wm_adsp.c: In function 'wm_adsp_load_coeff':
   sound/soc/codecs/wm_adsp.c:2250:5: warning: format '%d' expects argument of type 'int', but argument 8 has type 'size_t' [-Wformat]

vim +1705 sound/soc/codecs/wm_adsp.c

  1689				region_name = wm_adsp_mem_region_name(type);
  1690				reg = wm_adsp_region_to_reg(mem, offset);
  1691				break;
  1692			default:
  1693				adsp_warn(dsp,
  1694					  "%s.%d: Unknown region type %x at %d(%x)\n",
  1695					  file, regions, type, pos, pos);
  1696				break;
  1697			}
  1698	
  1699			adsp_dbg(dsp, "%s.%d: %d bytes at %d in %s\n", file,
  1700				 regions, le32_to_cpu(region->len), offset,
  1701				 region_name);
  1702	
  1703			if ((pos + le32_to_cpu(region->len) + sizeof(*region)) >
  1704			    firmware->size) {
> 1705				adsp_err(dsp,
  1706					 "%s.%d: %s region len %d bytes exceeds file length %d\n",
  1707					 file, regions, region_name,
  1708					 le32_to_cpu(region->len), firmware->size);
  1709				ret = -EINVAL;
  1710				goto out_fw;
  1711			}
  1712	
  1713			if (text) {

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

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ASoC: wm_adsp: Don't overrun firmware file buffer when reading region data
  2016-12-19 15:18 [PATCH] ASoC: wm_adsp: Don't overrun firmware file buffer when reading region data Richard Fitzgerald
  2016-12-19 19:16 ` kbuild test robot
@ 2016-12-19 19:20 ` kbuild test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2016-12-19 19:20 UTC (permalink / raw)
  To: Richard Fitzgerald; +Cc: kbuild-all, broonie, linux-kernel, alsa-devel, patches

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

Hi Richard,

[auto build test WARNING on asoc/for-next]
[also build test WARNING on v4.9 next-20161219]
[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/Richard-Fitzgerald/ASoC-wm_adsp-Don-t-overrun-firmware-file-buffer-when-reading-region-data/20161220-021733
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All warnings (new ones prefixed by >>):

   sound/soc/codecs/wm_adsp.c: In function 'wm_adsp_load':
>> sound/soc/codecs/wm_adsp.c:40:21: warning: format '%d' expects argument of type 'int', but argument 8 has type 'size_t {aka const long unsigned int}' [-Wformat=]
     dev_err(_dsp->dev, "DSP%d: " fmt, _dsp->num, ##__VA_ARGS__)
                        ^
>> sound/soc/codecs/wm_adsp.c:1705:4: note: in expansion of macro 'adsp_err'
       adsp_err(dsp,
       ^~~~~~~~
   sound/soc/codecs/wm_adsp.c: In function 'wm_adsp_load_coeff':
>> sound/soc/codecs/wm_adsp.c:40:21: warning: format '%d' expects argument of type 'int', but argument 8 has type 'size_t {aka const long unsigned int}' [-Wformat=]
     dev_err(_dsp->dev, "DSP%d: " fmt, _dsp->num, ##__VA_ARGS__)
                        ^
   sound/soc/codecs/wm_adsp.c:2250:5: note: in expansion of macro 'adsp_err'
        adsp_err(dsp,
        ^~~~~~~~

vim +40 sound/soc/codecs/wm_adsp.c

cdcd7f72 Charles Keepax      2014-11-14  24  #include <linux/vmalloc.h>
6ab2b7b4 Dimitris Papastamos 2013-05-08  25  #include <linux/workqueue.h>
f9f55e31 Richard Fitzgerald  2015-06-11  26  #include <linux/debugfs.h>
2159ad93 Mark Brown          2012-10-11  27  #include <sound/core.h>
2159ad93 Mark Brown          2012-10-11  28  #include <sound/pcm.h>
2159ad93 Mark Brown          2012-10-11  29  #include <sound/pcm_params.h>
2159ad93 Mark Brown          2012-10-11  30  #include <sound/soc.h>
2159ad93 Mark Brown          2012-10-11  31  #include <sound/jack.h>
2159ad93 Mark Brown          2012-10-11  32  #include <sound/initval.h>
2159ad93 Mark Brown          2012-10-11  33  #include <sound/tlv.h>
2159ad93 Mark Brown          2012-10-11  34  
2159ad93 Mark Brown          2012-10-11  35  #include "wm_adsp.h"
2159ad93 Mark Brown          2012-10-11  36  
2159ad93 Mark Brown          2012-10-11  37  #define adsp_crit(_dsp, fmt, ...) \
2159ad93 Mark Brown          2012-10-11  38  	dev_crit(_dsp->dev, "DSP%d: " fmt, _dsp->num, ##__VA_ARGS__)
2159ad93 Mark Brown          2012-10-11  39  #define adsp_err(_dsp, fmt, ...) \
2159ad93 Mark Brown          2012-10-11 @40  	dev_err(_dsp->dev, "DSP%d: " fmt, _dsp->num, ##__VA_ARGS__)
2159ad93 Mark Brown          2012-10-11  41  #define adsp_warn(_dsp, fmt, ...) \
2159ad93 Mark Brown          2012-10-11  42  	dev_warn(_dsp->dev, "DSP%d: " fmt, _dsp->num, ##__VA_ARGS__)
2159ad93 Mark Brown          2012-10-11  43  #define adsp_info(_dsp, fmt, ...) \
2159ad93 Mark Brown          2012-10-11  44  	dev_info(_dsp->dev, "DSP%d: " fmt, _dsp->num, ##__VA_ARGS__)
2159ad93 Mark Brown          2012-10-11  45  #define adsp_dbg(_dsp, fmt, ...) \
2159ad93 Mark Brown          2012-10-11  46  	dev_dbg(_dsp->dev, "DSP%d: " fmt, _dsp->num, ##__VA_ARGS__)
2159ad93 Mark Brown          2012-10-11  47  
2159ad93 Mark Brown          2012-10-11  48  #define ADSP1_CONTROL_1                   0x00

:::::: The code at line 40 was first introduced by commit
:::::: 2159ad936b7e7a8b26c99cf5b4476cfbb8c13e22 ASoC: adsp: Add ADSP base support

:::::: TO: Mark Brown <broonie@opensource.wolfsonmicro.com>
:::::: CC: Mark Brown <broonie@opensource.wolfsonmicro.com>

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

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-12-19 19:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-19 15:18 [PATCH] ASoC: wm_adsp: Don't overrun firmware file buffer when reading region data Richard Fitzgerald
2016-12-19 19:16 ` kbuild test robot
2016-12-19 19:20 ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).