* [PATCH v2 4/6] android/hal-audio: Fix audio with large omtu value
From: Andrzej Kaczmarek @ 2014-02-03 16:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1391446549-31455-1-git-send-email-andrzej.kaczmarek@tieto.com>
This patch fixes media packet construction with devices which use large
omtu value. In such cases it's possible that we will try to fit more
than 15 SBC frames in single media packet (which is maximum possible
value as it's encoded using 4 bits) which will cause frame counter to
wrap around and provide incorrect data to SBC encoder.
This behaviour was seen on UPF with one of carkit devices which set
omtu=2688.
---
android/hal-audio.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/android/hal-audio.c b/android/hal-audio.c
index ff2b6e4..be17c76 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -39,6 +39,8 @@
#define FIXED_A2DP_PLAYBACK_LATENCY_MS 25
+#define MAX_FRAMES_IN_PAYLOAD 15
+
static const uint8_t a2dp_src_uuid[] = {
0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
0x80, 0x00, 0x00, 0x80, 0x5f, 0x9b, 0x34, 0xfb };
@@ -483,7 +485,9 @@ static ssize_t sbc_write_data(void *codec_data, const void *buffer,
* input data
*/
if (mp->payload.frame_count == sbc_data->frames_per_packet ||
- bytes == consumed) {
+ bytes == consumed ||
+ mp->payload.frame_count ==
+ MAX_FRAMES_IN_PAYLOAD) {
ret = write_media_packet(fd, sbc_data, mp, encoded);
if (ret < 0)
return ret;
--
1.8.5.3
^ permalink raw reply related
* [PATCH v2 3/6] android/hal-audio: Print calculated SBC parameters
From: Andrzej Kaczmarek @ 2014-02-03 16:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1391446549-31455-1-git-send-email-andrzej.kaczmarek@tieto.com>
---
android/hal-audio.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/android/hal-audio.c b/android/hal-audio.c
index 1bf599c..ff2b6e4 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -332,6 +332,9 @@ static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
sbc_data->frames_per_packet = num_frames;
+ DBG("mtu=%u in_frame_len=%zu out_frame_len=%zu frames_per_packet=%zu",
+ mtu, in_frame_len, out_frame_len, num_frames);
+
*codec_data = sbc_data;
return AUDIO_STATUS_SUCCESS;
--
1.8.5.3
^ permalink raw reply related
* [PATCH v2 2/6] android/hal-audio: Be more verbose on SBC errors
From: Andrzej Kaczmarek @ 2014-02-03 16:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1391446549-31455-1-git-send-email-andrzej.kaczmarek@tieto.com>
---
android/hal-audio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/android/hal-audio.c b/android/hal-audio.c
index 4578c53..1bf599c 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -466,7 +466,7 @@ static ssize_t sbc_write_data(void *codec_data, const void *buffer,
&written);
if (ret < 0) {
- error("SBC: failed to encode block");
+ error("SBC: failed to encode block (%d)", ret);
break;
}
--
1.8.5.3
^ permalink raw reply related
* [PATCH v2 1/6] android/hal-audio: Remove unsupported mono channel mode
From: Andrzej Kaczmarek @ 2014-02-03 16:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1391446549-31455-1-git-send-email-andrzej.kaczmarek@tieto.com>
AudioFlinger can only provide PCM 16bit Stereo data for A2DP track so
we should not advertise mono channel mode in capabilities since we
can't downmix this internally.
---
android/hal-audio.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/android/hal-audio.c b/android/hal-audio.c
index 8ef5bff..4578c53 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -227,8 +227,7 @@ struct a2dp_audio_dev {
static const a2dp_sbc_t sbc_presets[] = {
{
.frequency = SBC_SAMPLING_FREQ_44100 | SBC_SAMPLING_FREQ_48000,
- .channel_mode = SBC_CHANNEL_MODE_MONO |
- SBC_CHANNEL_MODE_DUAL_CHANNEL |
+ .channel_mode = SBC_CHANNEL_MODE_DUAL_CHANNEL |
SBC_CHANNEL_MODE_STEREO |
SBC_CHANNEL_MODE_JOINT_STEREO,
.subbands = SBC_SUBBANDS_4 | SBC_SUBBANDS_8,
--
1.8.5.3
^ permalink raw reply related
* [PATCH v2 0/6] android/hal-audio: Fixes
From: Andrzej Kaczmarek @ 2014-02-03 16:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
v1 -> v2
fixed build error introduced during rebase
Andrzej Kaczmarek (6):
android/hal-audio: Remove unsupported mono channel mode
android/hal-audio: Be more verbose on SBC errors
android/hal-audio: Print calculated SBC parameters
android/hal-audio: Fix audio with large omtu value
android/hal-audio: Fix RTP sequence numbers
android/hal-audio: Add RTP timestamps
android/hal-audio.c | 33 +++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)
--
1.8.5.3
^ permalink raw reply
* How do you install bluez for development?
From: Alejandro Exojo @ 2014-02-03 16:21 UTC (permalink / raw)
To: linux-bluetooth
Hi.
I want to build a recent bluez, since I need the DualShock support.
I'm used to play with PATH, LD_LIBRARY_PATH and other stuff, so I tend
to install what I've built in /opt/foo or even in a subdirectory in my
/home, but I'm missing something, maybe obvious, to start bluetoothd.
I've built fine with ./configure --prefix=/ --enable-sixaxis &&
DESTDIR=/opt/bluez5 make install.
Then I've configured everything I saw that bluez installs:
* /opt/bluez5/etc/dbus-1/system.d/bluetooth.conf (sourced from the
stuff in /etc)
* /lib/udev (I've symlinked the two files, just in case)
* /etc/systemd/system/bluetooth.service (points to the service file
provided by bluez, with the proper path for bluetoothd).
With that the service starts, but stops immediately. This is what I've
get in the log when plugging the adapter (or forcing a manual
startup):
Feb 03 17:14:38 PC-MW03 systemd[1]: Starting Bluetooth service...
Feb 03 17:14:38 PC-MW03 bluetoothd[22571]: Bluetooth daemon 5.14
Feb 03 17:14:38 PC-MW03 systemd[1]: Started Bluetooth service.
Feb 03 17:14:38 PC-MW03 systemd[1]: Starting Bluetooth.
Feb 03 17:14:38 PC-MW03 systemd[1]: Reached target Bluetooth.
Feb 03 17:14:38 PC-MW03 bluetoothd[22571]: Failed to access management interface
Feb 03 17:14:38 PC-MW03 bluetoothd[22571]: Adapter handling
initialization failed
Feb 03 17:14:38 PC-MW03 systemd[1]: bluetooth.service: main process
exited, code=exited, status=1/FAILURE
Any hints? Or should I just give up and install everything in /usr,
possibly clashing files from other installations like packages?
Thank you.
--
Alejandro Exojo Piqueras
ModpoW, S.L.
Technova LaSalle | Sant Joan de la Salle 42 | 08022 Barcelona | www.modpow.es
^ permalink raw reply
* [PATCH] tools/rfcomm-tester: Fix double test pass for sock serv success TC
From: Grzegorz Kolodziejczyk @ 2014-02-03 15:56 UTC (permalink / raw)
To: linux-bluetooth
This patch fixes double set of pass state in Basic RFCOMM Socket Server
- Success test case. It avoid double test case teardown, which can
affect the proper conduct of later test cases. First set state to pass is
set within rfcomm_listen_cb. Second and properly at connection_cb.
---
tools/rfcomm-tester.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/rfcomm-tester.c b/tools/rfcomm-tester.c
index 44df7e7..9d2cfc6 100644
--- a/tools/rfcomm-tester.c
+++ b/tools/rfcomm-tester.c
@@ -436,6 +436,7 @@ static gboolean rfcomm_listen_cb(GIOChannel *io, GIOCondition cond,
gpointer user_data)
{
struct test_data *data = tester_get_data();
+ const struct rfcomm_server_data *server_data = data->test_data;
int sk, new_sk;
data->io_id = 0;
@@ -450,7 +451,8 @@ static gboolean rfcomm_listen_cb(GIOChannel *io, GIOCondition cond,
close(new_sk);
- tester_test_passed();
+ if (server_data->client_channel == 0x0e)
+ tester_test_passed();
return false;
}
--
1.8.5.2
^ permalink raw reply related
* Re: Working with the gatt-example.c plugin
From: Anderson Lizardo @ 2014-02-03 15:51 UTC (permalink / raw)
To: Sandy Chapman; +Cc: BlueZ development
In-Reply-To: <CAPm3yA3BzSHVG42RFp2U0m0DY5vKHVOY0gRwSOrZz8-qdsdzfQ@mail.gmail.com>
Hi Sandy,
On Mon, Feb 3, 2014 at 10:38 AM, Sandy Chapman <schapman@lixar.com> wrote:
> Hi All,
>
> I'm having some difficulties getting the gatt-example plugin working.
> I've checked out the latest tag (5.14) from git and have built it
> using the --enable-maintainer-mode configure flag so that
> gatt-example.c is compiled into a static plugin that is loaded at
> runtime. I've confirmed that the plugin is compiled and is loaded at
> runtime when I run bluetoothd.
>
> My problem is with making modifications to the plugin. I've tried
> something as simple as changing the UUID for the battery service,
> however, every time I scan the services (using an iPhone), the same
> list of UUIDs is returned (0xA002 , 0xA004, and 0xFEEE... for the
> services, which match up with what's defined in the gatt-example.c
> file). Any modifications I make to the file don't seem to have any
> effect on the services advertised. I've even rebuilt it without
> maintainer mode and have confirmed the plugin isn't loaded, but still,
> the same set of services and characteristics are loaded. Is there some
> sort of cache where the advertisement is loaded from that can be
> cleared? It looks like a bunch of HCI Commands are sent every time I
> "hciconfig hci0 up" which didn't happen when I initially ran the
> hciconfig tool prior to compiling and running bluetoothd.
Most likely iPhone is caching the attribute database and thus not
redoing any discovery. If this is the case, it is because we don't
provide a Service Changed characteristic on the GATT service,
therefore the iPhone thinks the database is static.
BTW this is a bug/limitation on the current BlueZ. While working on
the new GATT API we are also implementing a Service Changed service
that notifies changes to the entire database on every connection. This
is suboptimal (may trigger service discovery on every connection) but
at least will avoid stale cache on the client side. In future we need
to implement tracking of added / removed services and notify only
those that change.
Please use "btmon" while iphone connects to bluez and check if there
is any ATT traffic for GATT service discovery.
If this is the case, you need to find out how to clear the cache on the iPhone.
Best Regards,
--
Anderson Lizardo
http://www.indt.org/?lang=en
INdT - Manaus - Brazil
^ permalink raw reply
* [PATCH 6/6] android/hal-audio: Add RTP timestamps
From: Andrzej Kaczmarek @ 2014-02-03 15:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1391442292-4668-1-git-send-email-andrzej.kaczmarek@tieto.com>
---
android/hal-audio.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/android/hal-audio.c b/android/hal-audio.c
index ff42136..efdf823 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -133,6 +133,7 @@ struct sbc_data {
struct timespec start;
unsigned frames_sent;
+ uint32_t timestamp;
uint16_t seq;
};
@@ -404,6 +405,7 @@ static void sbc_resume(void *codec_data)
clock_gettime(CLOCK_MONOTONIC, &sbc_data->start);
sbc_data->frames_sent = 0;
+ sbc_data->timestamp = 0;
}
static int write_media_packet(int fd, struct sbc_data *sbc_data,
@@ -455,31 +457,38 @@ static ssize_t sbc_write_data(void *codec_data, const void *buffer,
struct media_packet *mp = (struct media_packet *) sbc_data->out_buf;
size_t free_space = sbc_data->out_buf_size - sizeof(*mp);
int ret;
+ ssize_t bytes_read;
mp->hdr.v = 2;
mp->hdr.pt = 1;
mp->hdr.ssrc = htonl(1);
+ mp->hdr.timestamp = htonl(sbc_data->timestamp);
mp->payload.frame_count = 0;
while (bytes - consumed >= sbc_data->in_frame_len) {
ssize_t written = 0;
- ret = sbc_encode(&sbc_data->enc, buffer + consumed,
+ bytes_read = sbc_encode(&sbc_data->enc, buffer + consumed,
sbc_data->in_frame_len,
mp->data + encoded, free_space,
&written);
- if (ret < 0) {
+ if (bytes_read < 0) {
error("SBC: failed to encode block (%zd)", bytes_read);
break;
}
mp->payload.frame_count++;
- consumed += ret;
+ consumed += bytes_read;
encoded += written;
free_space -= written;
+ /* AudioFlinger provides PCM 16bit stereo only, thus sample size
+ * is always 4 bytes
+ */
+ sbc_data->timestamp += (bytes_read / 4);
+
/* write data if we either filled media packed or encoded all
* input data
*/
@@ -495,6 +504,7 @@ static ssize_t sbc_write_data(void *codec_data, const void *buffer,
encoded = 0;
free_space = sbc_data->out_buf_size - sizeof(*mp);
+ mp->hdr.timestamp = htonl(sbc_data->timestamp);
mp->payload.frame_count = 0;
}
}
--
1.8.5.3
^ permalink raw reply related
* [PATCH 5/6] android/hal-audio: Fix RTP sequence numbers
From: Andrzej Kaczmarek @ 2014-02-03 15:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1391442292-4668-1-git-send-email-andrzej.kaczmarek@tieto.com>
---
android/hal-audio.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/android/hal-audio.c b/android/hal-audio.c
index eb064bc..ff42136 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -458,7 +458,6 @@ static ssize_t sbc_write_data(void *codec_data, const void *buffer,
mp->hdr.v = 2;
mp->hdr.pt = 1;
- mp->hdr.sequence_number = htons(sbc_data->seq++);
mp->hdr.ssrc = htonl(1);
mp->payload.frame_count = 0;
@@ -488,6 +487,8 @@ static ssize_t sbc_write_data(void *codec_data, const void *buffer,
bytes == consumed ||
mp->payload.frame_count ==
MAX_FRAMES_IN_PAYLOAD) {
+ mp->hdr.sequence_number = htons(sbc_data->seq++);
+
ret = write_media_packet(fd, sbc_data, mp, encoded);
if (ret < 0)
return ret;
--
1.8.5.3
^ permalink raw reply related
* [PATCH 4/6] android/hal-audio: Fix audio with large omtu value
From: Andrzej Kaczmarek @ 2014-02-03 15:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1391442292-4668-1-git-send-email-andrzej.kaczmarek@tieto.com>
This patch fixes media packet construction with devices which use large
omtu value. In such cases it's possible that we will try to fit more
than 15 SBC frames in single media packet (which is maximum possible
value as it's encoded using 4 bits) which will cause frame counter to
wrap around and provide incorrect data to SBC encoder.
This behaviour was seen on UPF with one of carkit devices which set
omtu=2688.
---
android/hal-audio.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/android/hal-audio.c b/android/hal-audio.c
index cda1359..eb064bc 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -39,6 +39,8 @@
#define FIXED_A2DP_PLAYBACK_LATENCY_MS 25
+#define MAX_FRAMES_IN_PAYLOAD 15
+
static const uint8_t a2dp_src_uuid[] = {
0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
0x80, 0x00, 0x00, 0x80, 0x5f, 0x9b, 0x34, 0xfb };
@@ -483,7 +485,9 @@ static ssize_t sbc_write_data(void *codec_data, const void *buffer,
* input data
*/
if (mp->payload.frame_count == sbc_data->frames_per_packet ||
- bytes == consumed) {
+ bytes == consumed ||
+ mp->payload.frame_count ==
+ MAX_FRAMES_IN_PAYLOAD) {
ret = write_media_packet(fd, sbc_data, mp, encoded);
if (ret < 0)
return ret;
--
1.8.5.3
^ permalink raw reply related
* [PATCH 3/6] android/hal-audio: Print calculated SBC parameters
From: Andrzej Kaczmarek @ 2014-02-03 15:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1391442292-4668-1-git-send-email-andrzej.kaczmarek@tieto.com>
---
android/hal-audio.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/android/hal-audio.c b/android/hal-audio.c
index bccaf15..cda1359 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -332,6 +332,9 @@ static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
sbc_data->frames_per_packet = num_frames;
+ DBG("mtu=%u in_frame_len=%zu out_frame_len=%zu frames_per_packet=%zu",
+ mtu, in_frame_len, out_frame_len, num_frames);
+
*codec_data = sbc_data;
return AUDIO_STATUS_SUCCESS;
--
1.8.5.3
^ permalink raw reply related
* [PATCH 2/6] android/hal-audio: Be more verbose on SBC errors
From: Andrzej Kaczmarek @ 2014-02-03 15:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1391442292-4668-1-git-send-email-andrzej.kaczmarek@tieto.com>
---
android/hal-audio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/android/hal-audio.c b/android/hal-audio.c
index 4578c53..bccaf15 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -466,7 +466,7 @@ static ssize_t sbc_write_data(void *codec_data, const void *buffer,
&written);
if (ret < 0) {
- error("SBC: failed to encode block");
+ error("SBC: failed to encode block (%zd)", bytes_read);
break;
}
--
1.8.5.3
^ permalink raw reply related
* [PATCH 1/6] android/hal-audio: Remove unsupported mono channel mode
From: Andrzej Kaczmarek @ 2014-02-03 15:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
AudioFlinger can only provide PCM 16bit Stereo data for A2DP track so
we should not advertise mono channel mode in capabilities since we
can't downmix this internally.
---
android/hal-audio.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/android/hal-audio.c b/android/hal-audio.c
index 8ef5bff..4578c53 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -227,8 +227,7 @@ struct a2dp_audio_dev {
static const a2dp_sbc_t sbc_presets[] = {
{
.frequency = SBC_SAMPLING_FREQ_44100 | SBC_SAMPLING_FREQ_48000,
- .channel_mode = SBC_CHANNEL_MODE_MONO |
- SBC_CHANNEL_MODE_DUAL_CHANNEL |
+ .channel_mode = SBC_CHANNEL_MODE_DUAL_CHANNEL |
SBC_CHANNEL_MODE_STEREO |
SBC_CHANNEL_MODE_JOINT_STEREO,
.subbands = SBC_SUBBANDS_4 | SBC_SUBBANDS_8,
--
1.8.5.3
^ permalink raw reply related
* Working with the gatt-example.c plugin
From: Sandy Chapman @ 2014-02-03 14:38 UTC (permalink / raw)
To: linux-bluetooth
Hi All,
I'm having some difficulties getting the gatt-example plugin working.
I've checked out the latest tag (5.14) from git and have built it
using the --enable-maintainer-mode configure flag so that
gatt-example.c is compiled into a static plugin that is loaded at
runtime. I've confirmed that the plugin is compiled and is loaded at
runtime when I run bluetoothd.
My problem is with making modifications to the plugin. I've tried
something as simple as changing the UUID for the battery service,
however, every time I scan the services (using an iPhone), the same
list of UUIDs is returned (0xA002 , 0xA004, and 0xFEEE... for the
services, which match up with what's defined in the gatt-example.c
file). Any modifications I make to the file don't seem to have any
effect on the services advertised. I've even rebuilt it without
maintainer mode and have confirmed the plugin isn't loaded, but still,
the same set of services and characteristics are loaded. Is there some
sort of cache where the advertisement is loaded from that can be
cleared? It looks like a bunch of HCI Commands are sent every time I
"hciconfig hci0 up" which didn't happen when I initially ran the
hciconfig tool prior to compiling and running bluetoothd.
Some additional details:
Bluetooth adapter is a Broadcom (in a MacBook Pro) that is running
through a Linux VM (VirtualBox). Additional diagnostics can be
provided on request.
Thanks for any help you guys can provide,
Sandy
--
^ permalink raw reply
* Re: [PATCHv3 1/5] android/unit: Add android IPC unit tests
From: Szymon Janc @ 2014-02-03 14:33 UTC (permalink / raw)
To: Marcin Kraglak; +Cc: linux-bluetooth
In-Reply-To: <1389866702-30697-1-git-send-email-marcin.kraglak@tieto.com>
Hi Marcin,
On Thursday 16 of January 2014 11:04:58 Marcin Kraglak wrote:
> It will test ipc library. First test case will check
> ipc_init() call.
> ---
> .gitignore | 1 +
> android/Makefile.am | 8 ++
> android/test-ipc.c | 277 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 286 insertions(+)
> create mode 100644 android/test-ipc.c
>
> diff --git a/.gitignore b/.gitignore
> index ac76fe2..ddd562a 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -114,3 +114,4 @@ android/bluetoothd
> android/haltest
> android/android-tester
> android/bluetoothd-snoop
> +android/test-ipc
> diff --git a/android/Makefile.am b/android/Makefile.am
> index cd4a526..0c61317 100644
> --- a/android/Makefile.am
> +++ b/android/Makefile.am
> @@ -117,6 +117,14 @@ android_android_tester_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
>
> android_android_tester_LDFLAGS = -pthread -ldl
>
> +noinst_PROGRAMS += android/test-ipc
> +
> +android_test_ipc_SOURCES = android/test-ipc.c \
> + src/shared/util.h src/shared/util.c \
> + src/log.h src/log.c \
> + android/ipc.c android/ipc.h
> +android_test_ipc_LDADD = @GLIB_LIBS@
> +
> plugin_LTLIBRARIES += android/audio.a2dp.default.la
>
> android_audio_a2dp_default_la_SOURCES = android/audio-msg.h \
> diff --git a/android/test-ipc.c b/android/test-ipc.c
> new file mode 100644
> index 0000000..6ac1175
> --- /dev/null
> +++ b/android/test-ipc.c
> @@ -0,0 +1,277 @@
> +/*
> + *
> + * BlueZ - Bluetooth protocol stack for Linux
> + *
> + * Copyright (C) 2014 Intel Corporation. All rights reserved.
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +#include <stdio.h>
> +#include <errno.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +#include <inttypes.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <sys/signalfd.h>
> +
> +#include <glib.h>
> +#include "src/shared/util.h"
> +#include "src/log.h"
> +#include "android/hal-msg.h"
> +#include "android/ipc.h"
> +
> +struct test_data {
> + uint32_t expected_signal;
> +};
> +
> +struct context {
> + GMainLoop *main_loop;
> +
> + int sk;
> +
> + guint source;
> + guint cmd_source;
> + guint notif_source;
> +
> + GIOChannel *cmd_io;
> + GIOChannel *notif_io;
> + GIOChannel *signal_io;
> +
> + guint signal_source;
> +
> + const struct test_data *data;
> +};
> +
> +static void context_quit(struct context *context)
> +{
> + g_main_loop_quit(context->main_loop);
> +}
> +
> +static gboolean cmd_watch(GIOChannel *io, GIOCondition cond,
> + gpointer user_data)
> +{
> + if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) {
> + g_assert(FALSE);
> + return FALSE;
> + }
> +
> + return TRUE;
> +}
> +
> +static gboolean notif_watch(GIOChannel *io, GIOCondition cond,
> + gpointer user_data)
> +{
> + if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) {
> + g_assert(FALSE);
> + return FALSE;
> + }
> +
> + return TRUE;
> +}
> +
> +static gboolean connect_handler(GIOChannel *io, GIOCondition cond,
> + gpointer user_data)
> +{
> + struct context *context = user_data;
> + GIOChannel *new_io;
> + GIOCondition watch_cond;
> + int sk;
> +
> + if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) {
> + g_assert(FALSE);
> + return FALSE;
> + }
> +
> + g_assert(!context->cmd_source || !context->notif_source);
> +
> + sk = accept(context->sk, NULL, NULL);
> + g_assert(sk >= 0);
> +
> + new_io = g_io_channel_unix_new(sk);
> +
> + watch_cond = G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL;
> +
> + if (context->cmd_source && !context->notif_source) {
> + context->notif_source = g_io_add_watch(new_io, watch_cond,
> + notif_watch, context);
> + g_assert(context->notif_source > 0);
> + context->notif_io = new_io;
> + }
> +
> + if (!context->cmd_source) {
> + context->cmd_source = g_io_add_watch(new_io, watch_cond,
> + cmd_watch, context);
> + context->cmd_io = new_io;
> + }
> +
> + if (context->cmd_source && context->notif_source)
> + context_quit(context);
> +
> + return TRUE;
> +}
> +
> +static gboolean signal_handler(GIOChannel *channel, GIOCondition cond,
> + gpointer user_data)
> +{
> + struct context *context = user_data;
> + const struct test_data *test_data = context->data;
> + struct signalfd_siginfo si;
> + ssize_t result;
> + int fd;
> +
> + if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP))
> + return FALSE;
> +
> + fd = g_io_channel_unix_get_fd(channel);
> +
> + result = read(fd, &si, sizeof(si));
> + if (result != sizeof(si))
> + return FALSE;
> +
> + g_assert(test_data->expected_signal == si.ssi_signo);
> + context_quit(context);
> + return TRUE;
> +}
> +
> +static guint setup_signalfd(gpointer user_data)
> +{
> + GIOChannel *channel;
> + guint source;
> + sigset_t mask;
> + int ret;
> + int fd;
> +
> + sigemptyset(&mask);
> + sigaddset(&mask, SIGINT);
> + sigaddset(&mask, SIGTERM);
> +
> + ret = sigprocmask(SIG_BLOCK, &mask, NULL);
> + g_assert(ret == 0);
> +
> + fd = signalfd(-1, &mask, 0);
> + g_assert(fd >= 0);
> +
> + channel = g_io_channel_unix_new(fd);
> +
> + g_io_channel_set_close_on_unref(channel, TRUE);
> + g_io_channel_set_encoding(channel, NULL, NULL);
> + g_io_channel_set_buffered(channel, FALSE);
> +
> + source = g_io_add_watch(channel,
> + G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
> + signal_handler, user_data);
> +
> + g_io_channel_unref(channel);
> +
> + return source;
> +}
> +
> +static struct context *create_context(gconstpointer data)
> +{
> + struct context *context = g_new0(struct context, 1);
> + struct sockaddr_un addr;
> + GIOChannel *io;
> + int ret, sk;
> +
> + context->main_loop = g_main_loop_new(NULL, FALSE);
> + g_assert(context->main_loop);
> +
> + context->signal_source = setup_signalfd(context);
> + g_assert(context->signal_source);
> +
> + sk = socket(AF_LOCAL, SOCK_SEQPACKET, 0);
> + g_assert(sk >= 0);
> +
> + memset(&addr, 0, sizeof(addr));
> + addr.sun_family = AF_UNIX;
> +
> + memcpy(addr.sun_path, BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH));
> +
> + ret = bind(sk, (struct sockaddr *) &addr, sizeof(addr));
> + g_assert(ret == 0);
> +
> + ret = listen(sk, 5);
> + g_assert(ret == 0);
> +
> + io = g_io_channel_unix_new(sk);
> +
> + g_io_channel_set_close_on_unref(io, TRUE);
> +
> + context->source = g_io_add_watch(io,
> + G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
> + connect_handler, context);
> + g_assert(context->source > 0);
> +
> + g_io_channel_unref(io);
> +
> + context->sk = sk;
> + context->data = data;
> +
> + return context;
> +}
> +
> +static void execute_context(struct context *context)
> +{
> + g_main_loop_run(context->main_loop);
> +
> + g_io_channel_shutdown(context->notif_io, true, NULL);
> + g_io_channel_shutdown(context->cmd_io, true, NULL);
> + g_io_channel_unref(context->cmd_io);
> + g_io_channel_unref(context->notif_io);
> +
> + g_source_remove(context->notif_source);
> + g_source_remove(context->signal_source);
> + g_source_remove(context->cmd_source);
> + g_source_remove(context->source);
> +
> + g_main_loop_unref(context->main_loop);
> +
> + g_free(context);
> +}
> +
> +static void test_init(gconstpointer data)
> +{
> + struct context *context = create_context(data);
> +
> + ipc_init();
> +
> + execute_context(context);
> +
> + ipc_cleanup();
> +}
> +
> +static const struct test_data test_init_1 = {};
> +
> +int main(int argc, char *argv[])
> +{
> + g_test_init(&argc, &argv, NULL);
> +
> + if (g_test_verbose())
> + __btd_log_init("*", 0);
> +
> + g_test_add_data_func("/android_ipc/init", &test_init_1, test_init);
> +
> + return g_test_run();
> +}
>
This is now applied (with some cleanups), thanks.
--
Best regards,
Szymon Janc
^ permalink raw reply
* Re: [RFC v6 03/14] Bluetooth: Introduce connection parameters list
From: Andre Guedes @ 2014-02-03 14:06 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: BlueZ development
In-Reply-To: <94043278-430F-4EAE-A4F1-36AB1FB614A6@holtmann.org>
Hi Marcel,
On Fri, 2014-01-31 at 17:11 -0800, Marcel Holtmann wrote:
> Hi Andre,
>
> > This patch adds to hdev the connection parameters list (hdev->le_
> > conn_params). The elements from this list (struct hci_conn_params)
> > contains the connection parameters (for now, minimum and maximum
> > connection interval) that should be used during the connection
> > establishment.
> >
> > The struct hci_conn_params also defines the 'auto_connect' field
> > which will be used to implement the auto connection mechanism.
> >
> > Moreover, this patch adds helper functions to manipulate hdev->le_
> > conn_params list. Some of these functions are also declared in
> > hci_core.h since they will be used outside hci_core.c in upcoming
> > patches.
> >
> > Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> > ---
> > include/net/bluetooth/hci_core.h | 25 +++++++++++++
> > net/bluetooth/hci_core.c | 80 ++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 105 insertions(+)
> >
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index 197413b..f757b3f 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -269,6 +269,7 @@ struct hci_dev {
> > struct list_head link_keys;
> > struct list_head long_term_keys;
> > struct list_head remote_oob_data;
> > + struct list_head le_conn_params;
> >
> > struct hci_dev_stats stat;
> >
> > @@ -373,6 +374,22 @@ struct hci_chan {
> > __u8 state;
> > };
> >
> > +struct hci_conn_params {
> > + struct list_head list;
> > +
> > + bdaddr_t addr;
> > + u8 addr_type;
> > +
> > + enum {
> > + HCI_AUTO_CONN_DISABLED,
> > + HCI_AUTO_CONN_ALWAYS,
> > + HCI_AUTO_CONN_LINK_LOSS,
> > + } auto_connect;
>
> actually I would not include the auto_connect mode in the this stage of the patch set. We could have made more progress with this patch set and get things applied if things are not intermixed. So lets leave this out and only introduce it once we need it.
All right, I'll introduce this auto_connect mode in patch 13/14. It
clearly makes more sense.
Thanks,
Andre
^ permalink raw reply
* BlueZ 5 mouse autoconnect
From: a.mv @ 2014-02-03 13:51 UTC (permalink / raw)
To: linux-bluetooth
I followed all possible guides to connect automatically my Apple
MagicMouse to my Linux PC. I saw on the forums that the key is to mark
it as "trusted". I did it in bluetoothctl and put it to "trusts" file in
/var/lib/bluetooth/xx:xx..../.
It works fine if I connect it manually but after restart I see only
messages like this in bluetoothctl:
[NEW] Device 84:38:35:24:08:D9 MagicMouse
[CHG] Device 84:38:35:24:08:D9 Class: 0x002580
[CHG] Device 84:38:35:24:08:D9 Icon: input-mouse
[CHG] Device 84:38:35:24:08:D9 Connected: yes
[CHG] Device 84:38:35:24:08:D9 Connected: no
[DEL] Device 84:38:35:24:08:D9 MagicMouse
[NEW] Device 84:38:35:24:08:D9 MagicMouse
[CHG] Device 84:38:35:24:08:D9 Class: 0x002580
[CHG] Device 84:38:35:24:08:D9 Icon: input-mouse
[CHG] Device 84:38:35:24:08:D9 Connected: yes
[CHG] Device 84:38:35:24:08:D9 Connected: no
[DEL] Device 84:38:35:24:08:D9 MagicMouse
Messages are printed when I click a mouse button. There is a small delay
between "Connected: yes" and "Connected: no" and during this time I see
a bluetooth sign in the notification area (I have Gnome).
Sometimes I see a paring request from the mouse. Sometimes authorisation
request and if I go to the bluetooth settings it even propose to type a
PIN code on the mouse )
Regardless my action after restart of bluez I am at the starting point.
^ permalink raw reply
* Re: [PATCH 1/3] android/pan: Fix control state change callback parameters order
From: Andrei Emeltchenko @ 2014-02-03 13:19 UTC (permalink / raw)
To: Ravi kumar Veeramally; +Cc: linux-bluetooth
In-Reply-To: <1390484367-6332-1-git-send-email-ravikumar.veeramally@linux.intel.com>
Hi Ravi,
On Thu, Jan 23, 2014 at 03:39:25PM +0200, Ravi kumar Veeramally wrote:
> Callback declared in bt_pan.h is
> 'typedef void (*btpan_control_state_callback)
> (btpan_control_state_t state, bt_status_t error, int local_role,
> const char* ifname);
>
> But PanService.Java defined it wrong way.
> private void onControlStateChanged(int local_role, int state,
> int error, String ifname).
> First and third parameters are misplaced, so sending data according
> to PanService.Java, discard this fix if issue fixed in PanService.Java.
> ---
> android/hal-pan.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/android/hal-pan.c b/android/hal-pan.c
> index 8c0f8d8..a596ffd 100644
> --- a/android/hal-pan.c
> +++ b/android/hal-pan.c
> @@ -45,9 +45,18 @@ static void handle_ctrl_state(void *buf, uint16_t len)
> {
> struct hal_ev_pan_ctrl_state *ev = buf;
>
> + /* FIXME: Callback declared in bt_pan.h is 'typedef void
> + * (*btpan_control_state_callback)(btpan_control_state_t state,
> + * bt_status_t error, int local_role, const char* ifname);
> + * But PanService.Java defined it wrong way.
> + * private void onControlStateChanged(int local_role, int state,
> + * int error, String ifname).
> + * First and third parameters are misplaced, so sending data according
> + * to PanService.Java, fix this if issue fixed in PanService.Java.
Just noticed that bluedroid use this with different combination of
arguments :)
callback.control_state_cb(state, btpan_role, status, TAP_IF_NAME);
How this ended up working? Maybe because only error and last argument are
important at this moment.
Best regards
Andrei Emeltchenko
> + */
> if (cbs->control_state_cb)
> - cbs->control_state_cb(ev->state, ev->status,
> - ev->local_role, (char *)ev->name);
> + cbs->control_state_cb(ev->local_role, ev->state, ev->status,
> + (char *)ev->name);
> }
>
> /* handlers will be called from notification thread context,
> --
> 1.8.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] unit/avctp: Add connection establishment dummy tests
From: Andrei Emeltchenko @ 2014-02-03 12:23 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Add some tests checking that L2CAP connection is established, so they
are basically dummy tests.
---
unit/test-avctp.c | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/unit/test-avctp.c b/unit/test-avctp.c
index 041e0c0..c0d16a4 100644
--- a/unit/test-avctp.c
+++ b/unit/test-avctp.c
@@ -220,10 +220,8 @@ static struct context *create_context(uint16_t version, gconstpointer data)
return context;
}
-static void execute_context(struct context *context)
+static void destroy_context(struct context *context)
{
- g_main_loop_run(context->main_loop);
-
if (context->source > 0)
g_source_remove(context->source);
@@ -235,6 +233,13 @@ static void execute_context(struct context *context)
g_free(context);
}
+static void execute_context(struct context *context)
+{
+ g_main_loop_run(context->main_loop);
+
+ destroy_context(context);
+}
+
static void test_client(gconstpointer data)
{
struct context *context = create_context(0x0100, data);
@@ -253,6 +258,13 @@ static void test_server(gconstpointer data)
execute_context(context);
}
+static void test_dummy(gconstpointer data)
+{
+ struct context *context = create_context(0x0100, data);
+
+ destroy_context(context);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -260,6 +272,18 @@ int main(int argc, char *argv[])
if (g_test_verbose())
__btd_log_init("*", 0);
+ /* Connection Channel Management tests */
+
+ /*
+ * Tests are checking that IUT is able to request establishing
+ * channels, since we already have connection through socketpair
+ * the tests are dummy.
+ */
+ define_test("/TP/CCM/BV-01-C", test_dummy, raw_pdu(0x00));
+ define_test("/TP/CCM/BV-02-C", test_dummy, raw_pdu(0x00));
+ define_test("/TP/CCM/BV-03-C", test_dummy, raw_pdu(0x00));
+ define_test("/TP/CCM/BV-04-C", test_dummy, raw_pdu(0x00));
+
define_test("/TP/NFR/BV-01-C", test_client,
raw_pdu(0x00, 0x11, 0x0e, 0x00, 0x00, 0x00));
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH] Bluetooth: hidp: make sure input buffers are big enough
From: David Herrmann @ 2014-02-03 11:27 UTC (permalink / raw)
To: Jiri Kosina
Cc: Marcel Holtmann, open list:HID CORE LAYER,
linux-bluetooth@vger.kernel.org development, Gustavo F. Padovan
In-Reply-To: <alpine.LNX.2.00.1402031107310.14066@pobox.suse.cz>
Hi Jiri
On Mon, Feb 3, 2014 at 11:08 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Wed, 29 Jan 2014, David Herrmann wrote:
>
>> >> Due to various reasons I will not have access to any testing HW for the
>> >> upcoming 2-3 days, so if you can cook something up in that timeframe, it'd
>> >> be appreciated.
>> >>
>> >> Otherwise I'll be working on it by the end of this week.
>> >
>> > David,
>> >
>> > just got back to this, finally ... did you have time to work on this at
>> > all, or should I just start from scratch?
>>
>> Sorry, no. Fosdem is this weekend and I needed to get my code ready
>> for that. But I'll finally have time again next week.
>
> Okay, thanks. I then guess we should proceed with this bandaid (double
> allocation) for 3.14
It would be really nice if we could get the HIDP patch into 3.14-rc2
and backported to stable. There have been quite a bunch of reports now
and I dislike adding a GFP_ATOMIC allocation in HID core. I am back
home now and will try to make HID core honor the length, but that's
quite invasive and shouldn't be used for stable.
Sorry for the delay,
David
^ permalink raw reply
* Re: [PATCH] Bluetooth: hidp: make sure input buffers are big enough
From: Jiri Kosina @ 2014-02-03 10:08 UTC (permalink / raw)
To: David Herrmann
Cc: Marcel Holtmann, open list:HID CORE LAYER,
linux-bluetooth@vger.kernel.org development, Gustavo F. Padovan
In-Reply-To: <CANq1E4TW+GU=oaW0tXb=Tun9HMtr26u-PC3H6BsLot560ANgog@mail.gmail.com>
On Wed, 29 Jan 2014, David Herrmann wrote:
> >> Due to various reasons I will not have access to any testing HW for the
> >> upcoming 2-3 days, so if you can cook something up in that timeframe, it'd
> >> be appreciated.
> >>
> >> Otherwise I'll be working on it by the end of this week.
> >
> > David,
> >
> > just got back to this, finally ... did you have time to work on this at
> > all, or should I just start from scratch?
>
> Sorry, no. Fosdem is this weekend and I needed to get my code ready
> for that. But I'll finally have time again next week.
Okay, thanks. I then guess we should proceed with this bandaid (double
allocation) for 3.14
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] avrcp: Fix wrong pointer check
From: Andrei Emeltchenko @ 2014-02-03 9:42 UTC (permalink / raw)
To: Luiz Augusto von Dentz, linux-bluetooth@vger.kernel.org
In-Reply-To: <20140203093651.GE2930@aemeltch-MOBL1>
On Mon, Feb 03, 2014 at 11:36:52AM +0200, Andrei Emeltchenko wrote:
> On Mon, Feb 03, 2014 at 01:20:11AM -0800, Luiz Augusto von Dentz wrote:
> > Hi Andrei,
> >
> > On Sun, Feb 2, 2014 at 11:51 PM, Andrei Emeltchenko
> > <Andrei.Emeltchenko.news@gmail.com> wrote:
> > > Hi Luiz,
> > >
> > > On Sun, Feb 02, 2014 at 08:03:34AM -0800, Luiz Augusto von Dentz wrote:
> > >> Hi Andrei,
> > >>
> > >> On Fri, Jan 31, 2014 at 12:33 AM, Andrei Emeltchenko
> > >> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > >> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > >> >
> > >> > There is wrong assumption that handler might be NULL while it is a
> > >> > pointer to a struct table so check instead for struct members. This
> > >> > fixes accessing wrong memory.
> > >> > ---
> > >> > profiles/audio/avrcp.c | 4 ++--
> > >> > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >> >
> > >> > diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> > >> > index df88138..5030ce1 100644
> > >> > --- a/profiles/audio/avrcp.c
> > >> > +++ b/profiles/audio/avrcp.c
> > >> > @@ -1673,7 +1673,7 @@ static size_t handle_vendordep_pdu(struct avctp *conn, uint8_t transaction,
> > >> > break;
> > >> > }
> > >> >
> > >> > - if (!handler || handler->code != *code) {
> > >> > + if (!handler->code || handler->code != *code) {
> > >>
> > >> The code checks if session->control_handlers is initialized and Im
> > >> not sure what is the invalid memory access you are talking about since
> > >> handle->code is no a pointer, I do agree that we should probably drop
> > >> the second check for the handler in the lines bellow.
> > >
> > > handler is a pointer, and it points to
> > >
> > > static const struct control_pdu_handler control_handlers[]
> > > table with the last element:
> > >
> > > ...
> > > { },
> > > };
> > > ...
> > >
> > > So checking for !handler is pointless.
> >
> > Right, because checking for pointer is pointless, yes Im being
> > sarcastic here... Now lets be clear, you are changing a check of a
> > pointer to a value and claiming it fixes invalid accesses which does
> > not make any sense, what could make sense is to check if
> > handler->pdu_id == pdu->pdu_id since that what we check when we lookup
> > for a handle.
>
> No, we break if handler->pdu_id == pdu->pdu_id one line above ;)
Sorry this was wrong. So my idea is to check that we are not at the end of
the table. In this case every element of a struct is zero, so it does not
matter which one to check.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* [PATCH BlueZ] audio/AVRCP: Fix checking for handler pointer instead of pdu_id
From: Luiz Augusto von Dentz @ 2014-02-03 9:40 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
The code may not find a valid handler for the pdu_id, in that case the
handler would not be NULL because the handlers table is not NULL
terminated, instead the code should check if pdu_id really matches.
---
profiles/audio/avrcp.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 12f7faa..4532d85 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -1673,7 +1673,7 @@ static size_t handle_vendordep_pdu(struct avctp *conn, uint8_t transaction,
break;
}
- if (!handler || handler->code != *code) {
+ if (handler->pdu_id != pdu->pdu_id || handler->code != *code) {
pdu->params[0] = AVRCP_STATUS_INVALID_COMMAND;
goto err_metadata;
}
@@ -1734,12 +1734,12 @@ static size_t handle_browsing_pdu(struct avctp *conn,
for (handler = browsing_handlers; handler->pdu_id; handler++) {
if (handler->pdu_id == pdu->pdu_id)
- break;
+ goto done;
}
- if (handler == NULL || handler->func == NULL)
- return avrcp_browsing_general_reject(operands);
+ return avrcp_browsing_general_reject(operands);
+done:
session->transaction = transaction;
handler->func(session, pdu, transaction);
return AVRCP_BROWSING_HEADER_LENGTH + ntohs(pdu->param_len);
--
1.8.4.2
^ permalink raw reply related
* Re: [PATCH] avrcp: Fix wrong pointer check
From: Andrei Emeltchenko @ 2014-02-03 9:36 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZ+VPAkfgUNRu6GC9DhkP=KXpTzBC03OVW8nNC_55uaMfg@mail.gmail.com>
On Mon, Feb 03, 2014 at 01:20:11AM -0800, Luiz Augusto von Dentz wrote:
> Hi Andrei,
>
> On Sun, Feb 2, 2014 at 11:51 PM, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > Hi Luiz,
> >
> > On Sun, Feb 02, 2014 at 08:03:34AM -0800, Luiz Augusto von Dentz wrote:
> >> Hi Andrei,
> >>
> >> On Fri, Jan 31, 2014 at 12:33 AM, Andrei Emeltchenko
> >> <Andrei.Emeltchenko.news@gmail.com> wrote:
> >> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >> >
> >> > There is wrong assumption that handler might be NULL while it is a
> >> > pointer to a struct table so check instead for struct members. This
> >> > fixes accessing wrong memory.
> >> > ---
> >> > profiles/audio/avrcp.c | 4 ++--
> >> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> >> > index df88138..5030ce1 100644
> >> > --- a/profiles/audio/avrcp.c
> >> > +++ b/profiles/audio/avrcp.c
> >> > @@ -1673,7 +1673,7 @@ static size_t handle_vendordep_pdu(struct avctp *conn, uint8_t transaction,
> >> > break;
> >> > }
> >> >
> >> > - if (!handler || handler->code != *code) {
> >> > + if (!handler->code || handler->code != *code) {
> >>
> >> The code checks if session->control_handlers is initialized and Im
> >> not sure what is the invalid memory access you are talking about since
> >> handle->code is no a pointer, I do agree that we should probably drop
> >> the second check for the handler in the lines bellow.
> >
> > handler is a pointer, and it points to
> >
> > static const struct control_pdu_handler control_handlers[]
> > table with the last element:
> >
> > ...
> > { },
> > };
> > ...
> >
> > So checking for !handler is pointless.
>
> Right, because checking for pointer is pointless, yes Im being
> sarcastic here... Now lets be clear, you are changing a check of a
> pointer to a value and claiming it fixes invalid accesses which does
> not make any sense, what could make sense is to check if
> handler->pdu_id == pdu->pdu_id since that what we check when we lookup
> for a handle.
No, we break if handler->pdu_id == pdu->pdu_id one line above ;)
Best regards
Andrei Emeltchenko
>
>
>
> --
> Luiz Augusto von Dentz
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox