[PATCH] Fix left overs handling in bluetooth_a2dp_write() of audio/pcm_bluetooth.c

[PATCH] Fix left overs handling in bluetooth_a2dp_write() of audio/pcm_bluetooth.c

[Colin Didier]

Hello,

If for some reason there is not enough data provided to the function
bluetooth_a2dp_write() and there are left overs to handle, the ALSA
module will segfault.

The test at the line 1053 of audio/pcm_bluetooth.c is wrong because it
compares unsigned integers and will lead to an unsigned integer overflow
if bytes_left is inferior to a2dp->codesize - data->count.

Here is a patch to fix this issue.

Cheers,
  Colin DIDIER


--- a/audio/pcm_bluetooth.c	2010-08-25 09:35:28.000000000 +0200
+++ b/audio/pcm_bluetooth.c	2010-08-25 09:37:47.000000000 +0200
@@ -1050,8 +1050,10 @@
 	}
 
 	/* Check if we have any left over data from the last write */
-	if (data->count > 0 && (bytes_left - data->count) >= a2dp->codesize) {
-		int additional_bytes_needed = a2dp->codesize - data->count;
+        if (data->count > 0) {
+                int additional_bytes_needed = a2dp->codesize - data->count;
+                if (additional_bytes_needed > bytes_left)
+                        goto out;
 
 		memcpy(data->buffer + data->count, buff,
 						additional_bytes_needed);
@@ -1122,6 +1124,7 @@
 		}
 	}
 
+out:
 	/* Copy the extra to our temp buffer for the next write */
 	if (bytes_left > 0) {
 		memcpy(data->buffer + data->count, buff, bytes_left);

Re: [PATCH] Fix left overs handling in bluetooth_a2dp_write() of audio/pcm_bluetooth.c

[Johan Hedberg]

Hi Colin,

On Wed, Aug 25, 2010, Colin Didier wrote:
> If for some reason there is not enough data provided to the function
> bluetooth_a2dp_write() and there are left overs to handle, the ALSA
> module will segfault.
> 
> The test at the line 1053 of audio/pcm_bluetooth.c is wrong because it
> compares unsigned integers and will lead to an unsigned integer overflow
> if bytes_left is inferior to a2dp->codesize - data->count.
> 
> Here is a patch to fix this issue.

Thanks for the patch, however it doesn't compile cleanly:

audio/pcm_bluetooth.c: In function ‘bluetooth_a2dp_write’:
audio/pcm_bluetooth.c:1055: error: comparison between signed and unsigned integer expressions
make[1]: *** [audio/audio_libasound_module_pcm_bluetooth_la-pcm_bluetooth.lo] Error 1
make: *** [all] Error 2

Always make sure that patches compile cleanly with
./bootstrap-configure. Also, could you please use git format-patch to
create the patches and make sure they have properly formated commit
messages (fits within an 80-column terminal with git log, not formated
like an email, etc). Thanks.

Johan

[PATCH] Fix leftovers handling

[Colin Didier]

If for some reason there is not enough data provided to the function
bluetooth_a2dp_write() and there are leftovers to handle, the ALSA
module will segfault.
---
 audio/pcm_bluetooth.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/audio/pcm_bluetooth.c b/audio/pcm_bluetooth.c
index 4c0ab6f..ff463fe 100644
--- a/audio/pcm_bluetooth.c
+++ b/audio/pcm_bluetooth.c
@@ -1050,8 +1050,11 @@ static snd_pcm_sframes_t bluetooth_a2dp_write(snd_pcm_ioplug_t *io,
 	}
 
 	/* Check if we have any left over data from the last write */
-	if (data->count > 0 && (bytes_left - data->count) >= a2dp->codesize) {
-		int additional_bytes_needed = a2dp->codesize - data->count;
+	if (data->count > 0) {
+		unsigned int additional_bytes_needed =
+						a2dp->codesize - data->count;
+		if (additional_bytes_needed > bytes_left)
+			goto out;
 
 		memcpy(data->buffer + data->count, buff,
 						additional_bytes_needed);
@@ -1122,6 +1125,7 @@ static snd_pcm_sframes_t bluetooth_a2dp_write(snd_pcm_ioplug_t *io,
 		}
 	}
 
+out:
 	/* Copy the extra to our temp buffer for the next write */
 	if (bytes_left > 0) {
 		memcpy(data->buffer + data->count, buff, bytes_left);
-- 
1.7.1

Re: [PATCH] Fix leftovers handling

[Johan Hedberg]

Hi Colin,

On Wed, Aug 25, 2010, Colin Didier wrote:
> If for some reason there is not enough data provided to the function
> bluetooth_a2dp_write() and there are leftovers to handle, the ALSA
> module will segfault.
> ---
>  audio/pcm_bluetooth.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)

Thanks. This patch is now upstream.

Johan