[alsa-devel] Bug in ALSA-utils/arecord with --max-file-time
Hi All,
I seem to have found a minor bug in aplay, within alsa-utils 1.1.4. It's possible that I'm the first person who's ever hit it. This appears to be the correct place to report it; if not, I humbly apologize, and ask for direction on where to submit this.
In aplay.c, around line 3030, we do this calculation:
max_file_size = max_file_time * snd_pcm_format_size(hwparams.format, hwparams.rate * hwparams.channels);
We do this to convert max_file_time from the '--max-file-time' parameter (an int) into max_file_size, a file length in bytes (a long long). Since snd_pcm_format_size() may return something 32-bit, this might overflow 32-bits when we do the multiplication, wasting the extra bytes of max_file_size.
On my test case on an armv7l system, a 1800-second file at 192 KHz recorded in S24_3LE doesn't overflow, but recording in S32_LE does. I accidentally found an edge case. In case anyone wants to know, the failure mode is that arecord will repeatedly create 44 byte .wav files very quickly, since these are larger than the negative max file size. This continues until you have a directory full of thousands of 44-byte files. Hilarious.
A simple change to cast max_file_time to a long long fixes this in my test case. There might be some cleaner solution, but this at least worked for me.
max_file_size = (long long) max_file_time * snd_pcm_format_size(hwparams.format, hwparams.rate * hwparams.channels);
Hopefully someone can vet this and carry it over to git. I've attached a patch file as well.
--
Additionally, while working to report this, I noticed that the bug tracker link on the sidebar of https://www.alsa-project.org/main/index.php/Main_Page is a dead link.
-Scott
On Fri, 23 Jun 2017 00:08:58 +0200, Gilliland, Scott M wrote:
Hi All,
I seem to have found a minor bug in aplay, within alsa-utils 1.1.4. It's possible that I'm the first person who's ever hit it. This appears to be the correct place to report it; if not, I humbly apologize, and ask for direction on where to submit this.
In aplay.c, around line 3030, we do this calculation:
max_file_size = max_file_time * snd_pcm_format_size(hwparams.format, hwparams.rate * hwparams.channels);We do this to convert max_file_time from the '--max-file-time' parameter (an int) into max_file_size, a file length in bytes (a long long). Since snd_pcm_format_size() may return something 32-bit, this might overflow 32-bits when we do the multiplication, wasting the extra bytes of max_file_size.
On my test case on an armv7l system, a 1800-second file at 192 KHz recorded in S24_3LE doesn't overflow, but recording in S32_LE does. I accidentally found an edge case. In case anyone wants to know, the failure mode is that arecord will repeatedly create 44 byte .wav files very quickly, since these are larger than the negative max file size. This continues until you have a directory full of thousands of 44-byte files. Hilarious.
A simple change to cast max_file_time to a long long fixes this in my test case. There might be some cleaner solution, but this at least worked for me.
max_file_size = (long long) max_file_time * snd_pcm_format_size(hwparams.format, hwparams.rate * hwparams.channels);Hopefully someone can vet this and carry it over to git. I've attached a patch file as well.
The change looks good. Could you post as a proper patch, i.e. with a commit log and your sign-off?
Additionally, while working to report this, I noticed that the bug tracker link on the sidebar of https://www.alsa-project.org/main/index.php/Main_Page is a dead link.
Yeah, it's a known problem for long time...
Takashi
How does this look? -- Scott Gilliland imtc.gatech.edu/people/scott-gilliland
From: Takashi Iwai tiwai@suse.de Sent: Friday, June 23, 2017 10:02:18 AM To: Gilliland, Scott M Cc: alsa-devel@alsa-project.org Subject: Re: [alsa-devel] Bug in ALSA-utils/arecord with --max-file-time On Fri, 23 Jun 2017 00:08:58 +0200, Gilliland, Scott M wrote:
Hi All,
I seem to have found a minor bug in aplay, within alsa-utils 1.1.4. It's possible that I'm the first person who's ever hit it. This appears to be the correct place to report it; if not, I humbly apologize, and ask for direction on where to submit this.
In aplay.c, around line 3030, we do this calculation:
max_file_size = max_file_time * snd_pcm_format_size(hwparams.format, hwparams.rate * hwparams.channels);
We do this to convert max_file_time from the '--max-file-time' parameter (an int) into max_file_size, a file length in bytes (a long long). Since snd_pcm_format_size() may return something 32-bit, this might overflow 32-bits when we do the multiplication, wasting the extra bytes of max_file_size.
On my test case on an armv7l system, a 1800-second file at 192 KHz recorded in S24_3LE doesn't overflow, but recording in S32_LE does. I accidentally found an edge case. In case anyone wants to know, the failure mode is that arecord will repeatedly create 44 byte .wav files very quickly, since these are larger than the negative max file size. This continues until you have a directory full of thousands of 44-byte files. Hilarious.
A simple change to cast max_file_time to a long long fixes this in my test case. There might be some cleaner solution, but this at least worked for me.
max_file_size = (long long) max_file_time * snd_pcm_format_size(hwparams.format, hwparams.rate * hwparams.channels);
Hopefully someone can vet this and carry it over to git. I've attached a patch file as well.
The change looks good. Could you post as a proper patch, i.e. with a commit log and your sign-off?
Additionally, while working to report this, I noticed that the bug tracker link on the sidebar of https://www.alsa-project.org/main/index.php/Main_Page is a dead link.
Yeah, it's a known problem for long time...
Takashi
On Fri, 23 Jun 2017 17:18:58 +0200, Gilliland, Scott M wrote:
How does this look?
Well, not quite good.
Please try to create a patch like for kernel, i.e. a subject line, and your full name as the author. And give Signed-off-by tag with your real name.
You'll have idea when you look through the other commits in alsa-lib.
thanks,
Takashi
-- Scott Gilliland imtc.gatech.edu/people/scott-gilliland
From: Takashi Iwai tiwai@suse.de Sent: Friday, June 23, 2017 10:02:18 AM To: Gilliland, Scott M Cc: alsa-devel@alsa-project.org Subject: Re: [alsa-devel] Bug in ALSA-utils/arecord with --max-file-time On Fri, 23 Jun 2017 00:08:58 +0200, Gilliland, Scott M wrote:
Hi All,
I seem to have found a minor bug in aplay, within alsa-utils 1.1.4. It's possible that I'm the first person who's ever hit it. This appears to be the correct place to report it; if not, I humbly apologize, and ask for direction on where to submit this.
In aplay.c, around line 3030, we do this calculation:
max_file_size = max_file_time * snd_pcm_format_size(hwparams.format, hwparams.rate * hwparams.channels);
We do this to convert max_file_time from the '--max-file-time' parameter (an int) into max_file_size, a file length in bytes (a long long). Since snd_pcm_format_size() may return something 32-bit, this might overflow 32-bits when we do the multiplication, wasting the extra bytes of max_file_size.
On my test case on an armv7l system, a 1800-second file at 192 KHz recorded in S24_3LE doesn't overflow, but recording in S32_LE does. I accidentally found an edge case. In case anyone wants to know, the failure mode is that arecord will repeatedly create 44 byte .wav files very quickly, since these are larger than the negative max file size. This continues until you have a directory full of thousands of 44-byte files. Hilarious.
A simple change to cast max_file_time to a long long fixes this in my test case. There might be some cleaner solution, but this at least worked for me.
max_file_size = (long long) max_file_time * snd_pcm_format_size(hwparams.format, hwparams.rate * hwparams.channels);
Hopefully someone can vet this and carry it over to git. I've attached a patch file as well.
The change looks good. Could you post as a proper patch, i.e. with a commit log and your sign-off?
Additionally, while working to report this, I noticed that the bug tracker link on the sidebar of https://www.alsa-project.org/main/index.php/Main_Page is a dead link.
Yeah, it's a known problem for long time...
Takashi
commit 698092eeff01d74056ec33ac1da74e407edf53e7 Author: Scott scott.gilliland@gatech.edu Date: Fri Jun 23 11:02:59 2017 -0400
Fixes a bug with --max-file-time where the file size could overflow 32 bits, causing thousands of tiny files.diff --git a/aplay/aplay.c b/aplay/aplay.c index f793c82..00af662 100644 --- a/aplay/aplay.c +++ b/aplay/aplay.c @@ -3027,7 +3027,7 @@ static void capture(char *orig_name) if (count == 0) count = LLONG_MAX; /* compute the number of bytes per file */
- max_file_size = max_file_time *
- max_file_size = (long long) max_file_time * snd_pcm_format_size(hwparams.format, hwparams.rate * hwparams.channels); /* WAVE-file should be even (I'm not sure), but wasting one byte
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
participants (2)
-
Gilliland, Scott M -
Takashi Iwai