diff options
author | Mark Brown <broonie@kernel.org> | 2024-07-11 01:26:23 +0200 |
---|---|---|
committer | Mark Brown <broonie@kernel.org> | 2024-07-11 01:26:23 +0200 |
commit | 1ee45e649ebf07f1438a464fc3bafd14becea797 (patch) | |
tree | a35d7e2e37f4a41bdcbf34db559428861decdcee /drivers/firmware | |
parent | ASoC: cs530x: Remove bclk from private structure (diff) | |
parent | firmware: cs_dsp: Rename fw_ver to wmfw_ver (diff) | |
download | linux-1ee45e649ebf07f1438a464fc3bafd14becea797.tar.xz linux-1ee45e649ebf07f1438a464fc3bafd14becea797.zip |
firmware: cs_dsp: Some small coding improvements
Merge series from Richard Fitzgerald <rf@opensource.cirrus.com>:
Commit series that makes some small improvements to code and the
kernel log messages.
Diffstat (limited to 'drivers/firmware')
-rw-r--r-- | drivers/firmware/cirrus/cs_dsp.c | 295 |
1 files changed, 185 insertions, 110 deletions
diff --git a/drivers/firmware/cirrus/cs_dsp.c b/drivers/firmware/cirrus/cs_dsp.c index d2aa0980ed78..a7b56eff086f 100644 --- a/drivers/firmware/cirrus/cs_dsp.c +++ b/drivers/firmware/cirrus/cs_dsp.c @@ -12,6 +12,7 @@ #include <linux/ctype.h> #include <linux/debugfs.h> #include <linux/delay.h> +#include <linux/minmax.h> #include <linux/module.h> #include <linux/moduleparam.h> #include <linux/seq_file.h> @@ -1050,7 +1051,7 @@ static int cs_dsp_create_control(struct cs_dsp *dsp, ctl->fw_name = dsp->fw_name; ctl->alg_region = *alg_region; - if (subname && dsp->fw_ver >= 2) { + if (subname && dsp->wmfw_ver >= 2) { ctl->subname_len = subname_len; ctl->subname = kasprintf(GFP_KERNEL, "%.*s", subname_len, subname); if (!ctl->subname) { @@ -1110,9 +1111,16 @@ struct cs_dsp_coeff_parsed_coeff { int len; }; -static int cs_dsp_coeff_parse_string(int bytes, const u8 **pos, const u8 **str) +static int cs_dsp_coeff_parse_string(int bytes, const u8 **pos, unsigned int avail, + const u8 **str) { - int length; + int length, total_field_len; + + /* String fields are at least one __le32 */ + if (sizeof(__le32) > avail) { + *pos = NULL; + return 0; + } switch (bytes) { case 1: @@ -1125,10 +1133,16 @@ static int cs_dsp_coeff_parse_string(int bytes, const u8 **pos, const u8 **str) return 0; } + total_field_len = ((length + bytes) + 3) & ~0x03; + if ((unsigned int)total_field_len > avail) { + *pos = NULL; + return 0; + } + if (str) *str = *pos + bytes; - *pos += ((length + bytes) + 3) & ~0x03; + *pos += total_field_len; return length; } @@ -1153,71 +1167,134 @@ static int cs_dsp_coeff_parse_int(int bytes, const u8 **pos) return val; } -static inline void cs_dsp_coeff_parse_alg(struct cs_dsp *dsp, const u8 **data, - struct cs_dsp_coeff_parsed_alg *blk) +static int cs_dsp_coeff_parse_alg(struct cs_dsp *dsp, + const struct wmfw_region *region, + struct cs_dsp_coeff_parsed_alg *blk) { const struct wmfw_adsp_alg_data *raw; + unsigned int data_len = le32_to_cpu(region->len); + unsigned int pos; + const u8 *tmp; - switch (dsp->fw_ver) { + raw = (const struct wmfw_adsp_alg_data *)region->data; + + switch (dsp->wmfw_ver) { case 0: case 1: - raw = (const struct wmfw_adsp_alg_data *)*data; - *data = raw->data; + if (sizeof(*raw) > data_len) + return -EOVERFLOW; blk->id = le32_to_cpu(raw->id); blk->name = raw->name; - blk->name_len = strlen(raw->name); + blk->name_len = strnlen(raw->name, ARRAY_SIZE(raw->name)); blk->ncoeff = le32_to_cpu(raw->ncoeff); + + pos = sizeof(*raw); break; default: - blk->id = cs_dsp_coeff_parse_int(sizeof(raw->id), data); - blk->name_len = cs_dsp_coeff_parse_string(sizeof(u8), data, + if (sizeof(raw->id) > data_len) + return -EOVERFLOW; + + tmp = region->data; + blk->id = cs_dsp_coeff_parse_int(sizeof(raw->id), &tmp); + pos = tmp - region->data; + + tmp = ®ion->data[pos]; + blk->name_len = cs_dsp_coeff_parse_string(sizeof(u8), &tmp, data_len - pos, &blk->name); - cs_dsp_coeff_parse_string(sizeof(u16), data, NULL); - blk->ncoeff = cs_dsp_coeff_parse_int(sizeof(raw->ncoeff), data); + if (!tmp) + return -EOVERFLOW; + + pos = tmp - region->data; + cs_dsp_coeff_parse_string(sizeof(u16), &tmp, data_len - pos, NULL); + if (!tmp) + return -EOVERFLOW; + + pos = tmp - region->data; + if (sizeof(raw->ncoeff) > (data_len - pos)) + return -EOVERFLOW; + + blk->ncoeff = cs_dsp_coeff_parse_int(sizeof(raw->ncoeff), &tmp); + pos += sizeof(raw->ncoeff); break; } + if ((int)blk->ncoeff < 0) + return -EOVERFLOW; + cs_dsp_dbg(dsp, "Algorithm ID: %#x\n", blk->id); cs_dsp_dbg(dsp, "Algorithm name: %.*s\n", blk->name_len, blk->name); cs_dsp_dbg(dsp, "# of coefficient descriptors: %#x\n", blk->ncoeff); + + return pos; } -static inline void cs_dsp_coeff_parse_coeff(struct cs_dsp *dsp, const u8 **data, - struct cs_dsp_coeff_parsed_coeff *blk) +static int cs_dsp_coeff_parse_coeff(struct cs_dsp *dsp, + const struct wmfw_region *region, + unsigned int pos, + struct cs_dsp_coeff_parsed_coeff *blk) { const struct wmfw_adsp_coeff_data *raw; + unsigned int data_len = le32_to_cpu(region->len); + unsigned int blk_len, blk_end_pos; const u8 *tmp; - int length; - switch (dsp->fw_ver) { + raw = (const struct wmfw_adsp_coeff_data *)®ion->data[pos]; + if (sizeof(raw->hdr) > (data_len - pos)) + return -EOVERFLOW; + + blk_len = le32_to_cpu(raw->hdr.size); + if (blk_len > S32_MAX) + return -EOVERFLOW; + + if (blk_len > (data_len - pos - sizeof(raw->hdr))) + return -EOVERFLOW; + + blk_end_pos = pos + sizeof(raw->hdr) + blk_len; + + blk->offset = le16_to_cpu(raw->hdr.offset); + blk->mem_type = le16_to_cpu(raw->hdr.type); + + switch (dsp->wmfw_ver) { case 0: case 1: - raw = (const struct wmfw_adsp_coeff_data *)*data; - *data = *data + sizeof(raw->hdr) + le32_to_cpu(raw->hdr.size); + if (sizeof(*raw) > (data_len - pos)) + return -EOVERFLOW; - blk->offset = le16_to_cpu(raw->hdr.offset); - blk->mem_type = le16_to_cpu(raw->hdr.type); blk->name = raw->name; - blk->name_len = strlen(raw->name); + blk->name_len = strnlen(raw->name, ARRAY_SIZE(raw->name)); blk->ctl_type = le16_to_cpu(raw->ctl_type); blk->flags = le16_to_cpu(raw->flags); blk->len = le32_to_cpu(raw->len); break; default: - tmp = *data; - blk->offset = cs_dsp_coeff_parse_int(sizeof(raw->hdr.offset), &tmp); - blk->mem_type = cs_dsp_coeff_parse_int(sizeof(raw->hdr.type), &tmp); - length = cs_dsp_coeff_parse_int(sizeof(raw->hdr.size), &tmp); - blk->name_len = cs_dsp_coeff_parse_string(sizeof(u8), &tmp, + pos += sizeof(raw->hdr); + tmp = ®ion->data[pos]; + blk->name_len = cs_dsp_coeff_parse_string(sizeof(u8), &tmp, data_len - pos, &blk->name); - cs_dsp_coeff_parse_string(sizeof(u8), &tmp, NULL); - cs_dsp_coeff_parse_string(sizeof(u16), &tmp, NULL); + if (!tmp) + return -EOVERFLOW; + + pos = tmp - region->data; + cs_dsp_coeff_parse_string(sizeof(u8), &tmp, data_len - pos, NULL); + if (!tmp) + return -EOVERFLOW; + + pos = tmp - region->data; + cs_dsp_coeff_parse_string(sizeof(u16), &tmp, data_len - pos, NULL); + if (!tmp) + return -EOVERFLOW; + + pos = tmp - region->data; + if (sizeof(raw->ctl_type) + sizeof(raw->flags) + sizeof(raw->len) > + (data_len - pos)) + return -EOVERFLOW; + blk->ctl_type = cs_dsp_coeff_parse_int(sizeof(raw->ctl_type), &tmp); + pos += sizeof(raw->ctl_type); blk->flags = cs_dsp_coeff_parse_int(sizeof(raw->flags), &tmp); + pos += sizeof(raw->flags); blk->len = cs_dsp_coeff_parse_int(sizeof(raw->len), &tmp); - - *data = *data + sizeof(raw->hdr) + length; break; } @@ -1227,6 +1304,8 @@ static inline void cs_dsp_coeff_parse_coeff(struct cs_dsp *dsp, const u8 **data, cs_dsp_dbg(dsp, "\tCoefficient flags: %#x\n", blk->flags); cs_dsp_dbg(dsp, "\tALSA control type: %#x\n", blk->ctl_type); cs_dsp_dbg(dsp, "\tALSA control len: %#x\n", blk->len); + + return blk_end_pos; } static int cs_dsp_check_coeff_flags(struct cs_dsp *dsp, @@ -1250,12 +1329,16 @@ static int cs_dsp_parse_coeff(struct cs_dsp *dsp, struct cs_dsp_alg_region alg_region = {}; struct cs_dsp_coeff_parsed_alg alg_blk; struct cs_dsp_coeff_parsed_coeff coeff_blk; - const u8 *data = region->data; - int i, ret; + int i, pos, ret; + + pos = cs_dsp_coeff_parse_alg(dsp, region, &alg_blk); + if (pos < 0) + return pos; - cs_dsp_coeff_parse_alg(dsp, &data, &alg_blk); for (i = 0; i < alg_blk.ncoeff; i++) { - cs_dsp_coeff_parse_coeff(dsp, &data, &coeff_blk); + pos = cs_dsp_coeff_parse_coeff(dsp, region, pos, &coeff_blk); + if (pos < 0) + return pos; switch (coeff_blk.ctl_type) { case WMFW_CTL_TYPE_BYTES: @@ -1324,6 +1407,10 @@ static unsigned int cs_dsp_adsp1_parse_sizes(struct cs_dsp *dsp, const struct wmfw_adsp1_sizes *adsp1_sizes; adsp1_sizes = (void *)&firmware->data[pos]; + if (sizeof(*adsp1_sizes) > firmware->size - pos) { + cs_dsp_err(dsp, "%s: file truncated\n", file); + return 0; + } cs_dsp_dbg(dsp, "%s: %d DM, %d PM, %d ZM\n", file, le32_to_cpu(adsp1_sizes->dm), le32_to_cpu(adsp1_sizes->pm), @@ -1340,6 +1427,10 @@ static unsigned int cs_dsp_adsp2_parse_sizes(struct cs_dsp *dsp, const struct wmfw_adsp2_sizes *adsp2_sizes; adsp2_sizes = (void *)&firmware->data[pos]; + if (sizeof(*adsp2_sizes) > firmware->size - pos) { + cs_dsp_err(dsp, "%s: file truncated\n", file); + return 0; + } cs_dsp_dbg(dsp, "%s: %d XM, %d YM %d PM, %d ZM\n", file, le32_to_cpu(adsp2_sizes->xm), le32_to_cpu(adsp2_sizes->ym), @@ -1379,12 +1470,10 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware, struct regmap *regmap = dsp->regmap; unsigned int pos = 0; const struct wmfw_header *header; - const struct wmfw_adsp1_sizes *adsp1_sizes; const struct wmfw_footer *footer; const struct wmfw_region *region; const struct cs_dsp_region *mem; const char *region_name; - char *text = NULL; struct cs_dsp_buf *buf; unsigned int reg; int regions = 0; @@ -1395,10 +1484,8 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware, ret = -EINVAL; - pos = sizeof(*header) + sizeof(*adsp1_sizes) + sizeof(*footer); - if (pos >= firmware->size) { - cs_dsp_err(dsp, "%s: file too short, %zu bytes\n", - file, firmware->size); + if (sizeof(*header) >= firmware->size) { + ret = -EOVERFLOW; goto out_fw; } @@ -1415,8 +1502,7 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware, goto out_fw; } - cs_dsp_info(dsp, "Firmware version: %d\n", header->ver); - dsp->fw_ver = header->ver; + dsp->wmfw_ver = header->ver; if (header->core != dsp->type) { cs_dsp_err(dsp, "%s: invalid core %d != %d\n", @@ -1426,33 +1512,47 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware, pos = sizeof(*header); pos = dsp->ops->parse_sizes(dsp, file, pos, firmware); + if ((pos == 0) || (sizeof(*footer) > firmware->size - pos)) { + ret = -EOVERFLOW; + goto out_fw; + } footer = (void *)&firmware->data[pos]; pos += sizeof(*footer); if (le32_to_cpu(header->len) != pos) { - cs_dsp_err(dsp, "%s: unexpected header length %d\n", - file, le32_to_cpu(header->len)); + ret = -EOVERFLOW; goto out_fw; } - cs_dsp_dbg(dsp, "%s: timestamp %llu\n", file, - le64_to_cpu(footer->timestamp)); + cs_dsp_info(dsp, "%s: format %d timestamp %#llx\n", file, header->ver, + le64_to_cpu(footer->timestamp)); + + while (pos < firmware->size) { + /* Is there enough data for a complete block header? */ + if (sizeof(*region) > firmware->size - pos) { + ret = -EOVERFLOW; + goto out_fw; + } - while (pos < firmware->size && - sizeof(*region) < firmware->size - pos) { region = (void *)&(firmware->data[pos]); + + if (le32_to_cpu(region->len) > firmware->size - pos - sizeof(*region)) { + ret = -EOVERFLOW; + goto out_fw; + } + region_name = "Unknown"; reg = 0; - text = NULL; offset = le32_to_cpu(region->offset) & 0xffffff; type = be32_to_cpu(region->type) & 0xff; switch (type) { + case WMFW_INFO_TEXT: case WMFW_NAME_TEXT: - region_name = "Firmware name"; - text = kzalloc(le32_to_cpu(region->len) + 1, - GFP_KERNEL); + region_name = "Info/Name"; + cs_dsp_info(dsp, "%s: %.*s\n", file, + min(le32_to_cpu(region->len), 100), region->data); break; case WMFW_ALGORITHM_DATA: region_name = "Algorithm"; @@ -1460,11 +1560,6 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware, if (ret != 0) goto out_fw; break; - case WMFW_INFO_TEXT: - region_name = "Information"; - text = kzalloc(le32_to_cpu(region->len) + 1, - GFP_KERNEL); - break; case WMFW_ABSOLUTE: region_name = "Absolute"; reg = offset; @@ -1498,23 +1593,6 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware, regions, le32_to_cpu(region->len), offset, region_name); - if (le32_to_cpu(region->len) > - firmware->size - pos - sizeof(*region)) { - cs_dsp_err(dsp, - "%s.%d: %s region len %d bytes exceeds file length %zu\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)); - cs_dsp_info(dsp, "%s: %s\n", file, text); - kfree(text); - text = NULL; - } - if (reg) { buf = cs_dsp_buf_alloc(region->data, le32_to_cpu(region->len), @@ -1556,7 +1634,9 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware, out_fw: regmap_async_complete(regmap); cs_dsp_buf_free(&buf_list); - kfree(text); + + if (ret == -EOVERFLOW) + cs_dsp_err(dsp, "%s: file content overflows file data\n", file); return ret; } @@ -1702,7 +1782,7 @@ static struct cs_dsp_alg_region *cs_dsp_create_region(struct cs_dsp *dsp, list_add_tail(&alg_region->list, &dsp->alg_regions); - if (dsp->fw_ver > 0) + if (dsp->wmfw_ver > 0) cs_dsp_ctl_fixup_base(dsp, alg_region); return alg_region; @@ -1825,7 +1905,7 @@ static int cs_dsp_adsp1_setup_algs(struct cs_dsp *dsp) ret = PTR_ERR(alg_region); goto out; } - if (dsp->fw_ver == 0) { + if (dsp->wmfw_ver == 0) { if (i + 1 < n_algs) { len = be32_to_cpu(adsp1_alg[i + 1].dm); len -= be32_to_cpu(adsp1_alg[i].dm); @@ -1847,7 +1927,7 @@ static int cs_dsp_adsp1_setup_algs(struct cs_dsp *dsp) ret = PTR_ERR(alg_region); goto out; } - if (dsp->fw_ver == 0) { + if (dsp->wmfw_ver == 0) { if (i + 1 < n_algs) { len = be32_to_cpu(adsp1_alg[i + 1].zm); len -= be32_to_cpu(adsp1_alg[i].zm); @@ -1938,7 +2018,7 @@ static int cs_dsp_adsp2_setup_algs(struct cs_dsp *dsp) ret = PTR_ERR(alg_region); goto out; } - if (dsp->fw_ver == 0) { + if (dsp->wmfw_ver == 0) { if (i + 1 < n_algs) { len = be32_to_cpu(adsp2_alg[i + 1].xm); len -= be32_to_cpu(adsp2_alg[i].xm); @@ -1960,7 +2040,7 @@ static int cs_dsp_adsp2_setup_algs(struct cs_dsp *dsp) ret = PTR_ERR(alg_region); goto out; } - if (dsp->fw_ver == 0) { + if (dsp->wmfw_ver == 0) { if (i + 1 < n_algs) { len = be32_to_cpu(adsp2_alg[i + 1].ym); len -= be32_to_cpu(adsp2_alg[i].ym); @@ -1982,7 +2062,7 @@ static int cs_dsp_adsp2_setup_algs(struct cs_dsp *dsp) ret = PTR_ERR(alg_region); goto out; } - if (dsp->fw_ver == 0) { + if (dsp->wmfw_ver == 0) { if (i + 1 < n_algs) { len = be32_to_cpu(adsp2_alg[i + 1].zm); len -= be32_to_cpu(adsp2_alg[i].zm); @@ -2086,7 +2166,6 @@ static int cs_dsp_load_coeff(struct cs_dsp *dsp, const struct firmware *firmware struct cs_dsp_alg_region *alg_region; const char *region_name; int ret, pos, blocks, type, offset, reg, version; - char *text = NULL; struct cs_dsp_buf *buf; if (!firmware) @@ -2125,10 +2204,20 @@ static int cs_dsp_load_coeff(struct cs_dsp *dsp, const struct firmware *firmware pos = le32_to_cpu(hdr->len); blocks = 0; - while (pos < firmware->size && - sizeof(*blk) < firmware->size - pos) { + while (pos < firmware->size) { + /* Is there enough data for a complete block header? */ + if (sizeof(*blk) > firmware->size - pos) { + ret = -EOVERFLOW; + goto out_fw; + } + blk = (void *)(&firmware->data[pos]); + if (le32_to_cpu(blk->len) > firmware->size - pos - sizeof(*blk)) { + ret = -EOVERFLOW; + goto out_fw; + } + type = le16_to_cpu(blk->type); offset = le16_to_cpu(blk->offset); version = le32_to_cpu(blk->ver) >> 8; @@ -2145,7 +2234,8 @@ static int cs_dsp_load_coeff(struct cs_dsp *dsp, const struct firmware *firmware region_name = "Unknown"; switch (type) { case (WMFW_NAME_TEXT << 8): - text = kzalloc(le32_to_cpu(blk->len) + 1, GFP_KERNEL); + cs_dsp_info(dsp, "%s: %.*s\n", dsp->fw_name, + min(le32_to_cpu(blk->len), 100), blk->data); break; case (WMFW_INFO_TEXT << 8): case (WMFW_METADATA << 8): @@ -2217,25 +2307,7 @@ static int cs_dsp_load_coeff(struct cs_dsp *dsp, const struct firmware *firmware break; } - if (text) { - memcpy(text, blk->data, le32_to_cpu(blk->len)); - cs_dsp_info(dsp, "%s: %s\n", dsp->fw_name, text); - kfree(text); - text = NULL; - } - if (reg) { - if (le32_to_cpu(blk->len) > - firmware->size - pos - sizeof(*blk)) { - cs_dsp_err(dsp, - "%s.%d: %s region len %d bytes exceeds file length %zu\n", - file, blocks, region_name, - le32_to_cpu(blk->len), - firmware->size); - ret = -EINVAL; - goto out_fw; - } - buf = cs_dsp_buf_alloc(blk->data, le32_to_cpu(blk->len), &buf_list); @@ -2274,7 +2346,10 @@ static int cs_dsp_load_coeff(struct cs_dsp *dsp, const struct firmware *firmware out_fw: regmap_async_complete(regmap); cs_dsp_buf_free(&buf_list); - kfree(text); + + if (ret == -EOVERFLOW) + cs_dsp_err(dsp, "%s: file content overflows file data\n", file); + return ret; } @@ -2337,8 +2412,8 @@ EXPORT_SYMBOL_NS_GPL(cs_dsp_adsp1_init, FW_CS_DSP); * Return: Zero for success, a negative number on error. */ int cs_dsp_adsp1_power_up(struct cs_dsp *dsp, - const struct firmware *wmfw_firmware, char *wmfw_filename, - const struct firmware *coeff_firmware, char *coeff_filename, + const struct firmware *wmfw_firmware, const char *wmfw_filename, + const struct firmware *coeff_firmware, const char *coeff_filename, const char *fw_name) { unsigned int val; @@ -2631,8 +2706,8 @@ static void cs_dsp_halo_stop_watchdog(struct cs_dsp *dsp) * Return: Zero for success, a negative number on error. */ int cs_dsp_power_up(struct cs_dsp *dsp, - const struct firmware *wmfw_firmware, char *wmfw_filename, - const struct firmware *coeff_firmware, char *coeff_filename, + const struct firmware *wmfw_firmware, const char *wmfw_filename, + const struct firmware *coeff_firmware, const char *coeff_filename, const char *fw_name) { int ret; |