2016-12-19 15:18:18

by Richard Fitzgerald

[permalink] [raw]
Subject: [PATCH] ASoC: wm_adsp: Don't overrun firmware file buffer when reading region data

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 <[email protected]>
---
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


2016-12-19 19:17:01

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] ASoC: wm_adsp: Don't overrun firmware file buffer when reading region data

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


Attachments:
(No filename) (2.19 kB)
.config.gz (45.29 kB)
Download all attachments

2016-12-19 19:21:03

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] ASoC: wm_adsp: Don't overrun firmware file buffer when reading region data

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 <[email protected]>
:::::: CC: Mark Brown <[email protected]>

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


Attachments:
(No filename) (4.02 kB)
.config.gz (46.76 kB)
Download all attachments