Summary: | ASTERISK-20858: app_minivm fails to clean up mkstemp files | ||||
Reporter: | Walter Doekes (wdoekes) | Labels: | |||
Date Opened: | 2013-01-03 08:33:27.000-0600 | Date Closed: | 2017-08-29 05:09:50 | ||
Priority: | Minor | Regression? | No | ||
Status: | Closed/Complete | Components: | Applications/app_minivm | ||
Versions: | Frequency of Occurrence | ||||
Related Issues: |
| ||||
Environment: | Attachments: | ||||
Description: | Matt recently touched this code and removed a comment about leaking fd's and files.
{noformat} tmpfd = mkstemp(newtmp); if (tmpfd < 0) { ast_log(LOG_WARNING, "Failed to create temporary file for volgain: %d\n", errno); ast_free(str1); ast_free(str2); return -1; } snprintf(tmpcmd, sizeof(tmpcmd), "sox -v %.4f %s.%s %s.%s", vmu->volgain, filename, format, newtmp, format); ast_safe_system(tmpcmd); {noformat} The fd leaks may be gone, but this looks like: (a) file leaks (b) misuse of mkstemp by appending format to the filename First we have mkstemp create an actual file "/tmp/ABCDEF" and then we write to "/tmp/ABCDEF.wav". No one cares about the original "/tmp/ABCDEF" on disk, and by writing to "/tmp/ABCDEF.wav" you're ignoring most of the functionality of mkstemp -- being secure and not overwriting existing files. Lastly, no one ever clears up fname (finalfilename.format), resulting in a second file leak. | ||||
Comments: | By: Matt Jordan (mjordan) 2013-01-03 08:47:28.120-0600 So, my only defense here is that I didn't introduce (a) or (b) :-) It looks like what you described is actual the original intent of the code - to use mkstemp for the unique string of characters it produces as an input to {{sox}}, as opposed to actually using the file it creates. You're right that it probably shouldn't do that and that it should at least clean up the 0 byte file it just created on disk. I don't think you're very likely to have a filename collision by appending '.wav' to ABCDEF when you call {{sox}}, but it is possible. By: Friendly Automation (friendly-automation) 2017-08-29 05:09:51.352-0500 Change 6312 merged by Jenkins2: voicemail: Fix various abuses of mkstemp [https://gerrit.asterisk.org/6312|https://gerrit.asterisk.org/6312] By: Friendly Automation (friendly-automation) 2017-08-29 05:17:48.622-0500 Change 6314 merged by Jenkins2: voicemail: Fix various abuses of mkstemp [https://gerrit.asterisk.org/6314|https://gerrit.asterisk.org/6314] By: Friendly Automation (friendly-automation) 2017-08-29 05:18:04.712-0500 Change 6313 merged by Jenkins2: voicemail: Fix various abuses of mkstemp [https://gerrit.asterisk.org/6313|https://gerrit.asterisk.org/6313] By: Friendly Automation (friendly-automation) 2017-08-29 05:19:25.273-0500 Change 6315 merged by Jenkins2: voicemail: Fix various abuses of mkstemp [https://gerrit.asterisk.org/6315|https://gerrit.asterisk.org/6315] |