Skip to content

igo_nr: bound host-supplied control and config data #10909

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

Open
lgirdwood wants to merge 3 commits into
base: main
Choose a base branch
from
+61 −27
Open

igo_nr: bound host-supplied control and config data #10909

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5d229e2
igo_nr: reject out-of-range control switch element count
lrgirdwo Jun 11, 2026
1405669
igo_nr: validate config blob size before copy
lrgirdwo Jun 11, 2026
23c7a52
igo_nr: compute circular buffer ends in samples not bytes
lrgirdwo Jun 11, 2026
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
88 changes: 61 additions & 27 deletions src/audio/igo_nr/igo_nr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ static int igo_nr_capture_s16(struct comp_data *cd,
struct sof_sink *sink,
int32_t frames)
{
int16_t *x, *y1, *y2;
int16_t *x_start, *y_start, *x_end, *y_end;
int16_t const *x, *x_start, *x_end;
int16_t *y1, *y2, *y_start, *y_end;
int16_t sample;
size_t x_size, y_size;
int x_size, y_size;
size_t request_size = frames * source_get_frame_bytes(source);
int sink_samples_without_wrap;
int samples_without_wrap;
Expand All @@ -93,12 +93,14 @@ static int igo_nr_capture_s16(struct comp_data *cd,

#endif

ret = source_get_data(source, request_size, (void const **)&x,
(void const **)&x_start, &x_size);
/* use the sample-count source/sink accessors so the returned sizes are
* already in samples and need no byte-to-element conversion
*/
ret = source_get_data_s16(source, request_size, &x, &x_start, &x_size);
if (ret)
return ret;

ret = sink_get_buffer(sink, request_size, (void **)&y1, (void **)&y_start, &y_size);
ret = sink_get_buffer_s16(sink, request_size, &y1, &y_start, &y_size);
if (ret) {
source_release_data(source, 0);
return ret;
Expand Down Expand Up @@ -130,7 +132,10 @@ static int igo_nr_capture_s16(struct comp_data *cd,
i++;
}

x = cir_buf_wrap(x, x_start, x_end);
/* x is const (read-only source), so wrap manually instead of
* cir_buf_wrap() which takes a non-const void *
*/
x = (x >= x_end) ? x - x_size : x;
y1 = cir_buf_wrap(y1, y_start, y_end);
samples_left -= samples_without_wrap;
}
Expand Down Expand Up @@ -172,9 +177,9 @@ static int igo_nr_capture_s24(struct comp_data *cd,
struct sof_sink *sink,
int32_t frames)
{
int32_t *x, *y1, *y2;
int32_t *x_start, *y_start, *x_end, *y_end;
size_t x_size, y_size;
int32_t const *x, *x_start, *x_end;
int32_t *y1, *y2, *y_start, *y_end;
int x_size, y_size;
size_t request_size = frames * source_get_frame_bytes(source);
int sink_samples_without_wrap;
int samples_without_wrap;
Expand All @@ -195,12 +200,14 @@ static int igo_nr_capture_s24(struct comp_data *cd,

#endif

ret = source_get_data(source, request_size, (void const **)&x,
(void const **)&x_start, &x_size);
/* use the sample-count source/sink accessors so the returned sizes are
* already in samples and need no byte-to-element conversion
*/
ret = source_get_data_s32(source, request_size, &x, &x_start, &x_size);
if (ret)
return ret;

ret = sink_get_buffer(sink, request_size, (void **)&y1, (void **)&y_start, &y_size);
ret = sink_get_buffer_s32(sink, request_size, &y1, &y_start, &y_size);
if (ret) {
source_release_data(source, 0);
return ret;
Expand Down Expand Up @@ -231,7 +238,10 @@ static int igo_nr_capture_s24(struct comp_data *cd,
i++;
}

x = cir_buf_wrap(x, x_start, x_end);
/* x is const (read-only source), so wrap manually instead of
* cir_buf_wrap() which takes a non-const void *
*/
x = (x >= x_end) ? x - x_size : x;
y1 = cir_buf_wrap(y1, y_start, y_end);
samples_left -= samples_without_wrap;
}
Expand Down Expand Up @@ -273,9 +283,9 @@ static int igo_nr_capture_s32(struct comp_data *cd,
struct sof_sink *sink,
int32_t frames)
{
int32_t *x, *y1, *y2;
int32_t *x_start, *y_start, *x_end, *y_end;
size_t x_size, y_size;
int32_t const *x, *x_start, *x_end;
int32_t *y1, *y2, *y_start, *y_end;
int x_size, y_size;
size_t request_size = frames * source_get_frame_bytes(source);
int sink_samples_without_wrap;
int samples_without_wrap;
Expand All @@ -296,12 +306,14 @@ static int igo_nr_capture_s32(struct comp_data *cd,

#endif

ret = source_get_data(source, request_size, (void const **)&x,
(void const **)&x_start, &x_size);
/* use the sample-count source/sink accessors so the returned sizes are
* already in samples and need no byte-to-element conversion
*/
ret = source_get_data_s32(source, request_size, &x, &x_start, &x_size);
if (ret)
return ret;

ret = sink_get_buffer(sink, request_size, (void **)&y1, (void **)&y_start, &y_size);
ret = sink_get_buffer_s32(sink, request_size, &y1, &y_start, &y_size);
if (ret) {
source_release_data(source, 0);
return ret;
Expand Down Expand Up @@ -332,7 +344,10 @@ static int igo_nr_capture_s32(struct comp_data *cd,
i++;
}

x = cir_buf_wrap(x, x_start, x_end);
/* x is const (read-only source), so wrap manually instead of
* cir_buf_wrap() which takes a non-const void *
*/
x = (x >= x_end) ? x - x_size : x;
y1 = cir_buf_wrap(y1, y_start, y_end);
samples_left -= samples_without_wrap;
}
Expand Down Expand Up @@ -566,6 +581,10 @@ static int igo_nr_get_config(struct processing_module *mod,
comp_info(dev, "SOF_CTRL_CMD_BINARY");
return comp_data_blob_get_cmd(cd->model_handler, cdata, fragment_size);
case SOF_CTRL_CMD_SWITCH:
if (cdata->num_elems > SOF_IPC_MAX_CHANNELS) {
comp_err(dev, "num_elems %u out of range", cdata->num_elems);
return -EINVAL;
}
for (j = 0; j < cdata->num_elems; j++) {
Comment thread
lgirdwood marked this conversation as resolved.
cdata->chanv[j].channel = j;
cdata->chanv[j].value = cd->process_enable[j];
Expand Down Expand Up @@ -715,17 +734,32 @@ static void igo_nr_print_config(struct processing_module *mod)
static void igo_nr_set_igo_params(struct processing_module *mod)
{
struct comp_data *cd = module_get_private_data(mod);
struct sof_igo_nr_config *p_config = comp_get_data_blob(cd->model_handler, NULL, NULL);
size_t config_size;
struct sof_igo_nr_config *p_config = comp_get_data_blob(cd->model_handler,
&config_size, NULL);

@lyakh lyakh Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

off by 1 indentation. comp_get_data_blob() already (IMHO dubiously) sets *size to 0 immediately on entry, so actually config_size initialisation is redundant. But yes, I think a better approach for passed by reference output parameters is to only set them when the function executes successfully. Unfortunately SOF doesn't have conventions for that.

@lgirdwood lgirdwood Jun 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the indentation via the de-nesting, and dropped the redundant = 0 initialiser on config_size (comp_get_data_blob() sets it on entry).

@lyakh lyakh Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 724 indentation still off

struct comp_dev *dev = mod->dev;

comp_info(dev, "entry");

/* Adopt the host blob only when new config is valid */
if (p_config && igo_nr_check_config_validity(dev, cd) == 0) {
comp_info(dev, "New config detected.");
cd->config = *p_config;
igo_nr_print_config(mod);
if (!p_config)
return;

/* the whole config struct is copied below, so the blob must be at
* least that large
*/
if (config_size < sizeof(cd->config)) {
comp_err(dev, "New config too small: %zu < %zu",
config_size, sizeof(cd->config));
return;
}

/* Adopt the host blob only when the new config is valid */
if (igo_nr_check_config_validity(dev, cd) != 0)
return;

comp_info(dev, "New config detected.");
cd->config = *p_config;
igo_nr_print_config(mod);
}

/* copy and process stream data from source to sink buffers */
Expand Down
Loading