Skip to content

Fix arithmetic overflows#2383

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
plbossart merged 2 commits into from
Aug 28, 2020
Merged

Fix arithmetic overflows #2383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
58f2fc1
ASoC: SOF: (cosmetic) remove redundant "ret" variable uses
lyakh Aug 28, 2020
852808e
ASoC: SOF: fix range checks
lyakh Aug 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 28 additions & 31 deletions sound/soc/sof/control.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -221,7 +221,6 @@ int snd_sof_bytes_get(struct snd_kcontrol *kcontrol,
struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
struct sof_abi_hdr *data = cdata->data;
size_t size;
int ret = 0;

if (be->max > sizeof(ucontrol->value.bytes.data)) {
dev_err_ratelimited(scomp->dev,
Expand All@@ -230,20 +229,20 @@ int snd_sof_bytes_get(struct snd_kcontrol *kcontrol,
return -EINVAL;
Comment thread
kv2019i marked this conversation as resolved.
}

size = data->size + sizeof(*data);
if (size > be->max) {
/* be->max has been verified to be >= sizeof(struct sof_abi_hdr) */
if (data->size > be->max - sizeof(*data)) {
dev_err_ratelimited(scomp->dev,
"error: DSP sent %zu bytes max is %d\n",
size, be->max);
ret = -EINVAL;
goto out;
"error: %u bytes of control data is invalid, max is %lu\n",
data->size, be->max - sizeof(*data));
Comment thread
ranj063 marked this conversation as resolved.
return -EINVAL;
}

size = data->size + sizeof(*data);

/* copy back to kcontrol */
memcpy(ucontrol->value.bytes.data, data, size);

out:
return ret;
return 0;
}

int snd_sof_bytes_put(struct snd_kcontrol *kcontrol,
Expand All@@ -255,7 +254,7 @@ int snd_sof_bytes_put(struct snd_kcontrol *kcontrol,
struct snd_soc_component *scomp = scontrol->scomp;
struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
struct sof_abi_hdr *data = cdata->data;
size_t size = data->size + sizeof(*data);
size_t size;

if (be->max > sizeof(ucontrol->value.bytes.data)) {
dev_err_ratelimited(scomp->dev,
Expand All@@ -264,13 +263,16 @@ int snd_sof_bytes_put(struct snd_kcontrol *kcontrol,
return -EINVAL;
}

if (size > be->max) {
/* be->max has been verified to be >= sizeof(struct sof_abi_hdr) */
if (data->size > be->max - sizeof(*data)) {
dev_err_ratelimited(scomp->dev,
"error: size too big %zu bytes max is %d\n",
size, be->max);
"error: data size too big %u bytes max is %lu\n",
data->size, be->max - sizeof(*data));
return -EINVAL;
}

size = data->size + sizeof(*data);

/* copy from kcontrol */
memcpy(data, ucontrol->value.bytes.data, size);

Expand DownExpand Up@@ -337,7 +339,8 @@ int snd_sof_bytes_ext_put(struct snd_kcontrol *kcontrol,
return -EINVAL;
}

if (cdata->data->size + sizeof(const struct sof_abi_hdr) > be->max) {
/* be->max has been verified to be >= sizeof(struct sof_abi_hdr) */
if (cdata->data->size > be->max - sizeof(const struct sof_abi_hdr)) {
dev_err_ratelimited(scomp->dev, "error: Mismatch in ABI data size (truncated?).\n");
return -EINVAL;
}
Expand DownExpand Up@@ -423,8 +426,7 @@ int snd_sof_bytes_ext_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_tlv header;
struct snd_ctl_tlv __user *tlvd =
(struct snd_ctl_tlv __user *)binary_data;
int data_size;
int ret = 0;
size_t data_size;

/*
* Decrement the limit by ext bytes header size to
Expand All@@ -436,27 +438,22 @@ int snd_sof_bytes_ext_get(struct snd_kcontrol *kcontrol,
cdata->data->magic = SOF_ABI_MAGIC;
cdata->data->abi = SOF_ABI_VERSION;

/* Prevent read of other kernel data or possibly corrupt response */
data_size = cdata->data->size + sizeof(const struct sof_abi_hdr);

/* check data size doesn't exceed max coming from topology */
if (data_size > be->max) {
dev_err_ratelimited(scomp->dev, "error: user data size %d exceeds max size %d.\n",
data_size, be->max);
ret = -EINVAL;
goto out;
if (cdata->data->size > be->max - sizeof(const struct sof_abi_hdr)) {
dev_err_ratelimited(scomp->dev, "error: user data size %d exceeds max size %lu.\n",
cdata->data->size, be->max - sizeof(const struct sof_abi_hdr));
return -EINVAL;
}

data_size = cdata->data->size + sizeof(const struct sof_abi_hdr);

header.numid = scontrol->cmd;
header.length = data_size;
if (copy_to_user(tlvd, &header, sizeof(const struct snd_ctl_tlv))) {
ret = -EFAULT;
goto out;
}
if (copy_to_user(tlvd, &header, sizeof(const struct snd_ctl_tlv)))
return -EFAULT;

if (copy_to_user(tlvd->tlv, cdata->data, data_size))
ret = -EFAULT;
return -EFAULT;

out:
return ret;
return 0;
}
44 changes: 24 additions & 20 deletions sound/soc/sof/topology.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -63,7 +63,7 @@ static int ipc_pcm_params(struct snd_sof_widget *swidget, int dir)
struct sof_ipc_pcm_params pcm;
struct snd_pcm_hw_params *params;
struct snd_sof_pcm *spcm;
int ret = 0;
int ret;

memset(&pcm, 0, sizeof(pcm));

Expand DownExpand Up@@ -121,7 +121,7 @@ static int ipc_trigger(struct snd_sof_widget *swidget, int cmd)
struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp);
struct sof_ipc_stream stream;
struct sof_ipc_reply reply;
int ret = 0;
int ret;

/* set IPC stream params */
stream.hdr.size = sizeof(stream);
Expand DownExpand Up@@ -1033,7 +1033,7 @@ static int sof_control_load_volume(struct snd_soc_component *scomp,
struct sof_ipc_ctrl_data *cdata;
int tlv[TLV_ITEMS];
unsigned int i;
int ret = 0;
int ret;

/* validate topology data */
if (le32_to_cpu(mc->num_channels) > SND_SOC_TPLG_MAX_CHAN) {
Expand DownExpand Up@@ -1098,7 +1098,7 @@ static int sof_control_load_volume(struct snd_soc_component *scomp,
dev_dbg(scomp->dev, "tplg: load kcontrol index %d chans %d\n",
scontrol->comp_id, scontrol->num_channels);

return ret;
return 0;

out_free_table:
if (le32_to_cpu(mc->max) > 1)
Expand DownExpand Up@@ -1150,20 +1150,26 @@ static int sof_control_load_bytes(struct snd_soc_component *scomp,
struct snd_soc_tplg_bytes_control *control =
container_of(hdr, struct snd_soc_tplg_bytes_control, hdr);
struct soc_bytes_ext *sbe = (struct soc_bytes_ext *)kc->private_value;
int max_size = sbe->max;
int ret = 0;
size_t max_size = sbe->max;
size_t priv_size = le32_to_cpu(control->priv.size);
int ret;

/* init the get/put bytes data */
scontrol->size = sizeof(struct sof_ipc_ctrl_data) +
le32_to_cpu(control->priv.size);
if (max_size < sizeof(struct sof_ipc_ctrl_data) ||
max_size < sizeof(struct sof_abi_hdr)) {
ret = -EINVAL;
goto out;
}

if (scontrol->size > max_size) {
dev_err(scomp->dev, "err: bytes data size %d exceeds max %d.\n",
scontrol->size, max_size);
/* init the get/put bytes data */
if (priv_size > max_size - sizeof(struct sof_ipc_ctrl_data)) {
dev_err(scomp->dev, "err: bytes data size %lu exceeds max %lu.\n",
priv_size, max_size - sizeof(struct sof_ipc_ctrl_data));
ret = -EINVAL;
goto out;
}

scontrol->size = sizeof(struct sof_ipc_ctrl_data) + priv_size;

scontrol->control_data = kzalloc(max_size, GFP_KERNEL);
cdata = scontrol->control_data;
if (!scontrol->control_data) {
Expand DownExpand Up@@ -1204,7 +1210,7 @@ static int sof_control_load_bytes(struct snd_soc_component *scomp,
}
}

return ret;
return 0;

out_free:
kfree(scontrol->control_data);
Expand All@@ -1223,7 +1229,7 @@ static int sof_control_load(struct snd_soc_component *scomp, int index,
struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp);
struct snd_soc_dobj *dobj;
struct snd_sof_control *scontrol;
int ret = -EINVAL;
int ret;

dev_dbg(scomp->dev, "tplg: load control type %d name : %s\n",
hdr->type, hdr->name);
Expand DownExpand Up@@ -1276,7 +1282,7 @@ static int sof_control_load(struct snd_soc_component *scomp, int index,

dobj->private = scontrol;
list_add(&scontrol->list, &sdev->kcontrol_list);
return ret;
return 0;
}

static int sof_control_unload(struct snd_soc_component *scomp,
Expand DownExpand Up@@ -2659,7 +2665,7 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index,
struct snd_soc_tplg_private *private = &pcm->priv;
struct snd_sof_pcm *spcm;
int stream;
int ret = 0;
int ret;

/* nothing to do for BEs atm */
if (!pcm)
Expand DownExpand Up@@ -3350,7 +3356,6 @@ static int sof_link_hda_unload(struct snd_sof_dev *sdev,
struct snd_soc_dai_link *link)
{
struct snd_soc_dai *dai;
int ret = 0;

dai = snd_soc_find_dai(link->cpus);
if (!dai) {
Expand All@@ -3359,7 +3364,7 @@ static int sof_link_hda_unload(struct snd_sof_dev *sdev,
return -EINVAL;
}

return ret;
return 0;
}

static int sof_link_unload(struct snd_soc_component *scomp,
Expand DownExpand Up@@ -3492,7 +3497,6 @@ static int sof_route_load(struct snd_soc_component *scomp, int index,
sink_swidget->id != snd_soc_dapm_buffer) {
dev_dbg(scomp->dev, "warning: neither Linked source component %s nor sink component %s is of buffer type, ignoring link\n",
route->source, route->sink);
ret = 0;
goto err;
} else {
ret = sof_ipc_tx_message(sdev->ipc,
Expand DownExpand Up@@ -3526,7 +3530,7 @@ static int sof_route_load(struct snd_soc_component *scomp, int index,
/* add route to route list */
list_add(&sroute->list, &sdev->route_list);

return ret;
return 0;
}

err:
Expand Down