Skip to content

[bug]: ast_frame_adjust_volume and ast_frame_adjust_volume_float crash on interpolated frames #1230

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

Jump to bottom
Open
1 task done
mkmer opened this issue May 1, 2025 · 1 comment · May be fixed by #1241
Open
1 task done

[bug]: ast_frame_adjust_volume and ast_frame_adjust_volume_float crash on interpolated frames #1230

mkmer opened this issue May 1, 2025 · 1 comment · May be fixed by #1241
Labels
bug support-level-core Functionality with core support level

Comments

@mkmer
Copy link
Contributor

mkmer commented May 1, 2025

Severity

Trivial

Versions

22.2.0

Components/Modules

audiohook

Operating Environment

Linux Hub01 6.1.0-34-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.135-1 (2025-04-25) x86_64 GNU/Linux

Frequency of Occurrence

Frequent

Issue Description

When this section of code generates a frame:

asterisk/main/translate.c

Lines 537 to 563 in 754dea3

static struct ast_frame *generate_interpolated_slin(struct ast_trans_pvt *p, struct ast_frame *f)
{
struct ast_frame res = { AST_FRAME_VOICE };
/*
* If we've gotten here then we should have an interpolated frame that was not handled
* by the translation codec. So create an interpolated frame in the appropriate format
* that was going to be written. This frame might be handled later by other resources.
* For instance, generic plc.
*
* Note, generic plc is currently only available for the format type 'slin' (8KHz only -
* The generic plc code appears to have been based around that). Generic plc is filled
* in later on frame write.
*/
if (!ast_opt_generic_plc || f->datalen != 0 ||
ast_format_cmp(p->explicit_dst, ast_format_slin) == AST_FORMAT_CMP_NOT_EQUAL) {
return NULL;
}
res.subclass.format = ast_format_cache_get_slin_by_rate(8000); /* ref bumped on dup */
res.samples = f->samples;
res.datalen = 0;
res.data.ptr = NULL;
res.offset = AST_FRIENDLY_OFFSET;
return ast_frdup(&res);
}

we have a frame with datalen == 0 and data.ptr == NULL.
When said frame arrives at the audiohook to adjust volume, this code only looks at samples

asterisk/main/frame.c

Lines 812 to 832 in 754dea3

int ast_frame_adjust_volume_float(struct ast_frame *f, float adjustment)
{
int count;
short *fdata = f->data.ptr;
float adjust_value = fabs(adjustment);
if ((f->frametype != AST_FRAME_VOICE) || !(ast_format_cache_is_slinear(f->subclass.format))) {
return -1;
}
if (!adjustment) {
return 0;
}
for (count = 0; count < f->samples; count++) {
if (adjustment > 0) {
ast_slinear_saturated_multiply_float(&fdata[count], &adjust_value);
} else if (adjustment < 0) {
ast_slinear_saturated_divide_float(&fdata[count], &adjust_value);
}
}

which has a NULL pointer in f->data.ptr -> often resulting in a segmentation fault in ast_slinear_saturated_multiply_float()

I'm not sure which side of this is "wrong" - should we set samples != 0 when there is no data?
At a minimum, the ast_frame_adjust_volume_float() should verify f->samples is not reaching outside of f->datalen

Relevant log output

https://github.com/AllStarLink/app_rpt/issues/666

Asterisk Issue Guidelines

  • Yes, I have read the Asterisk Issue Guidelines
@mkmer mkmer mentioned this issue May 1, 2025
Open
@jcolp
Copy link
Member

jcolp commented May 1, 2025

The code handling the resulting frame should not assume audio is present in if samples is set.

@jcolp jcolp added the support-level-core Functionality with core support level label May 5, 2025
@jcolp jcolp changed the title [bug]: ast_frame_adjust_volume and ast_frame_adjust_volume_float crash [bug]: ast_frame_adjust_volume and ast_frame_adjust_volume_float crash on interpolated frames May 5, 2025
@jcolp jcolp removed the triage label May 5, 2025
@mkmer mkmer mentioned this issue May 5, 2025
Draft
mkmer added a commit to mkmer/asterisk_core that referenced this issue May 12, 2025
@mkmer
frame.c: validate frame data length is less than samples when adjusti…
b9b17ac 
…ng volume

Resolves: asterisk#1230
mkmer added a commit to mkmer/asterisk_core that referenced this issue May 12, 2025
@mkmer
frame.c: validate frame data length is less than samples when adjusti…
0e45613 
…ng volume

Resolves: asterisk#1230
@mkmer mkmer linked a pull request May 12, 2025 that will close this issue
Open
mkmer added a commit to mkmer/asterisk_core that referenced this issue May 12, 2025
@mkmer
frame.c: validate frame data length is less than samples when adjusti…
7bd6849 
…ng volume

Resolves: asterisk#1230
mkmer added a commit to mkmer/asterisk_core that referenced this issue May 12, 2025
@mkmer
frame.c: validate frame data length is less than samples when adjusti…
fec1237 
…ng volume

Resolves: asterisk#1230
mkmer added a commit to mkmer/asterisk_core that referenced this issue May 12, 2025
@mkmer
frame.c: validate frame data length is less than samples when adjusti…
2c75f2e 
…ng volume

Resolves: asterisk#1230
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug support-level-core Functionality with core support level
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

frame.c: validate frame data length. mkmer/asterisk_core
2 participants
@jcolp @mkmer