Skip to content

Commit

Permalink
firmware: cs_dsp: Prevent buffer overrun when processing V2 alg headers
Browse files Browse the repository at this point in the history 
[ Upstream commit 2163aff ]

Check that all fields of a V2 algorithm header fit into the available
firmware data buffer.

The wmfw V2 format introduced variable-length strings in the algorithm
block header. This means the overall header length is variable, and the
position of most fields varies depending on the length of the string
fields. Each field must be checked to ensure that it does not overflow
the firmware data buffer.

As this ia bugfix patch, the fixes avoid making any significant change to
the existing code. This makes it easier to review and less likely to
introduce new bugs.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: f6bc909 ("firmware: cs_dsp: add driver to support firmware loading on Cirrus Logic DSPs")
Link: https://patch.msgid.link/20240627141432.93056-5-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
@rfvirgil
rfvirgil authored and Sasha Levin committed Jul 13, 2024
1 parent 2c7a08e commit 947652f
Showing 1 changed file with 113 additions and 31 deletions.
144 changes: 113 additions & 31 deletions drivers/firmware/cirrus/cs_dsp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1053,9 +1053,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:
Expand All @@ -1068,10 +1075,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;
}
Expand All @@ -1096,71 +1109,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;

raw = (const struct wmfw_adsp_alg_data *)region->data;

switch (dsp->fw_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->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 = &region->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;

raw = (const struct wmfw_adsp_coeff_data *)&region->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->fw_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->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 = &region->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;
}

Expand All @@ -1170,6 +1246,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,
Expand All @@ -1193,12 +1271,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:
Expand Down

0 comments on commit 947652f

Please sign in to comment.