Skip to contentSkip to conversation

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

PATCH In review Colin Didier 2 participants 4 messages Aug 25, 2010 Bluetooth drivers

lore
Share & tools
lore

Colin Didier

author
#0 lore raw

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

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

audio/pcm_bluetooth.c +5 -2
@@ -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);
#1 lore raw

Hi Colin,

Quoted 9 lines · Colin Didier

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

author
#2 lore raw

PATCH 1/1 Fix leftovers handling

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(-)
audio/pcm_bluetooth.c +6 -2
index 4c0ab6f..ff463fe 100644
@@ -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);
Signature · 1 line
1.7.1
#3 lore raw

Re: [PATCH] Fix leftovers handling

Hi Colin,

Quoted 6 lines · 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(-)

Thanks. This patch is now upstream.

Johan