* Re: [PATCH 3/8] Bluetooth: Fix checking for valid device class values
From: Marcel Holtmann @ 2013-01-09 20:07 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1357738180-4128-4-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> The two lowest bits of the minor device class value are reserved and
> should be zero, and the three highest bits of the major device class
> likewise. The management code should therefore test for this and return
> a proper "invalid params" error if the condition is not met.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index aaf9ce6..2785de2 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -1430,6 +1430,12 @@ static int set_dev_class(struct sock *sk, struct hci_dev *hdev, void *data,
> goto unlock;
> }
>
> + if ((cp->minor & 0x03) != 0 || (cp->major & 0xe0) != 0) {
> + err = cmd_status(sk, hdev->id, MGMT_OP_SET_DEV_CLASS,
> + MGMT_STATUS_INVALID_PARAMS);
> + goto unlock;
> + }
we could do
if ((cp->minor & 0x03) || ...) {
However I am not sure what is preferred and I am fine both ways.
> +
> hdev->major_class = cp->major;
> hdev->minor_class = cp->minor;
>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH BlueZ 1/3] unit: Reuse define_test() macro for /TP/SERVER/BRW/* tests
From: Anderson Lizardo @ 2013-01-09 20:06 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1357758529.1806.46.camel@aeonflux>
Hi Marcel,
On Wed, Jan 9, 2013 at 3:08 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> -#define define_test(name, args...) \
>> +#define define_test(name, _mtu, args...) \
>> do { \
>> const struct sdp_pdu pdus[] = { \
>> args, { }, { } \
>> }; \
>> static struct test_data data; \
>> - data.mtu = 48; \
>> + data.mtu = _mtu; \
>
> so using _mtu instead of mtu was the trick here. For the heck of it I
> could not figure that one out. So in the end I gave up on it and
> hardcoded it :(
Yes, I was caught by it as well, until I realized that the
preprocessor might be replacing the "mtu" part of "data.mtu" (I
confirmed when I saw the cpp output).
>
> Patch has been applied.
Thanks,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH 2/8] Bluetooth: Fix missing command complete for mgmt_load_long_term_keys
From: Marcel Holtmann @ 2013-01-09 20:04 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1357738180-4128-3-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> All management events are expected to indicate successful completion
> through a command complete event, however the load long term keys
> command was missing this. This patch adds the missing event.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/8] Bluetooth: Fix missing command complete event for mgmt_confirm_name
From: Marcel Holtmann @ 2013-01-09 20:02 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1357738180-4128-2-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> All management commands are expected to indicate successful completion
> through a command complete event however the confirm name command was
> missing it. This patch add the sending of the missing event.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] Bluetooth device 04ca:3008 should use ath3k
From: Gustavo Padovan @ 2013-01-09 19:53 UTC (permalink / raw)
To: Sergio Cambra; +Cc: linux-bluetooth
In-Reply-To: <10056676.fQr0UMXTN5@tablet>
Hi Sergio,
* Sergio Cambra <sergio@enpijama.es> [2013-01-04 00:22:29 +0100]:
> Output of /sys/kernel/debug/usb/devices
> T: Bus=03 Lev=02 Prnt=02 Port=00 Cnt=01 Dev#= 6 Spd=12 MxCh= 0
> D: Ver= 1.10 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs= 1
> P: Vendor=04ca ProdID=3008 Rev= 0.02
> C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
> I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E: Ad=81(I) Atr=03(Int.) MxPS= 16 Ivl=1ms
> E: Ad=82(I) Atr=02(Bulk) MxPS= 64 Ivl=0ms
> E: Ad=02(O) Atr=02(Bulk) MxPS= 64 Ivl=0ms
> I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E: Ad=83(I) Atr=01(Isoc) MxPS= 0 Ivl=1ms
> E: Ad=03(O) Atr=01(Isoc) MxPS= 0 Ivl=1ms
> I: If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E: Ad=83(I) Atr=01(Isoc) MxPS= 9 Ivl=1ms
> E: Ad=03(O) Atr=01(Isoc) MxPS= 9 Ivl=1ms
> I: If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E: Ad=83(I) Atr=01(Isoc) MxPS= 17 Ivl=1ms
> E: Ad=03(O) Atr=01(Isoc) MxPS= 17 Ivl=1ms
> I: If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E: Ad=83(I) Atr=01(Isoc) MxPS= 25 Ivl=1ms
> E: Ad=03(O) Atr=01(Isoc) MxPS= 25 Ivl=1ms
> I: If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E: Ad=83(I) Atr=01(Isoc) MxPS= 33 Ivl=1ms
> E: Ad=03(O) Atr=01(Isoc) MxPS= 33 Ivl=1ms
> I: If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E: Ad=83(I) Atr=01(Isoc) MxPS= 49 Ivl=1ms
> E: Ad=03(O) Atr=01(Isoc) MxPS= 49 Ivl=1ms
>
> Signed-off-by: Sergio Cambra <sergio@programatica.es>
> ---
> drivers/bluetooth/ath3k.c | 2 ++
> drivers/bluetooth/btusb.c | 1 +
> 2 files changed, 3 insertions(+)
Your patch is broken, please use git send-email to send it to the mailing
list.
Gustavo
^ permalink raw reply
* Re: [PATCH v2 BlueZ 3/3] unit: Implement tests for sdp_extract_attr()
From: Marcel Holtmann @ 2013-01-09 19:50 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1357744762-5111-3-git-send-email-anderson.lizardo@openbossa.org>
Hi Anderson,
> These tests are for valid data. Other tests might be added later to
> check for error paths, but will require separate macros.
>
> As example of failure output, if commit
> 504a0cf46ad89cab8005ce9cffb22e41048f6a30 is reverted for testing, the
> STR16 test will fail with:
>
> ERROR:unit/test-sdp.c:776:test_sdp_de_attr: assertion failed
> (test->input_size == size): (7 == 11)
> ---
> unit/test-sdp.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 65 insertions(+)
>
> diff --git a/unit/test-sdp.c b/unit/test-sdp.c
> index 77a4c6c..d511ef4 100644
> --- a/unit/test-sdp.c
> +++ b/unit/test-sdp.c
> @@ -85,6 +85,29 @@ struct test_data {
> #define define_ssa(name, args...) define_test("/TP/SERVER/SSA/" name, 48, args)
> #define define_brw(name, args...) define_test("/TP/SERVER/BRW/" name, 672, args)
>
> +/* SDP Data Element (DE) tests */
> +struct test_data_de {
> + const void *input_data;
> + size_t input_size;
> + sdp_data_t expected;
> +};
> +
> +#define exp_data(_dtd, val_type, val_data) \
> + ((const sdp_data_t) { \
> + .dtd = _dtd, \
> + .val.val_type = val_data, \
> + })
> +
> +#define define_test_de_attr(name, input, exp) \
> + do { \
> + static struct test_data_de data; \
> + data.input_data = input; \
> + data.input_size = sizeof(input); \
> + data.expected = exp; \
> + g_test_add_data_func("/sdp/DE/ATTR/" name, &data, \
> + test_sdp_de_attr); \
> + } while (0)
> +
> struct context {
> GMainLoop *main_loop;
> guint server_source;
> @@ -742,6 +765,35 @@ static void test_sdp(gconstpointer data)
> g_free(test->pdu_list);
> }
>
> +static void test_sdp_de_attr(gconstpointer data)
> +{
> + const struct test_data_de *test = data;
> + sdp_data_t *d;
> + int size = 0;
> +
> + d = sdp_extract_attr(test->input_data, test->input_size, &size, NULL);
> + g_assert(d != NULL);
> + g_assert_cmpuint(test->input_size, ==, size);
> + g_assert_cmpuint(test->expected.dtd, ==, d->dtd);
> +
> + if (g_test_verbose() == TRUE)
> + g_print("DTD=0x%02x\n", d->dtd);
> +
> + switch (d->dtd) {
> + case SDP_TEXT_STR8:
> + case SDP_TEXT_STR16:
> + case SDP_URL_STR8:
> + case SDP_URL_STR16:
> + g_assert_cmpstr(test->expected.val.str, ==, d->val.str);
> + break;
> + /* TODO: implement other DTDs here */
just remove both TODOs from the patch. I assume you will send updates on
this.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] Bluetooth: Fix incorrect strncpy() in hidp_setup_hid()
From: Gustavo Padovan @ 2013-01-09 19:40 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1357511333-5276-1-git-send-email-anderson.lizardo@openbossa.org>
Hi Anderson,
* Anderson Lizardo <anderson.lizardo@openbossa.org> [2013-01-06 18:28:53 -0400]:
> The length parameter should be sizeof(req->name) - 1 because there is no
> guarantee that string provided by userspace will contain the trailing
> '\0'.
>
> Can be easily reproduced by manually setting req->name to 128 non-zero
> bytes prior to ioctl(HIDPCONNADD) and checking the device name setup on
> input subsystem:
>
> $ cat /sys/devices/pnp0/00\:04/tty/ttyS0/hci0/hci0\:1/input8/name
> AAAAAA[...]AAAAAAAAf0:af:f0:af:f0:af
>
> ("f0:af:f0:af:f0:af" is the device bluetooth address, taken from "phys"
> field in struct hid_device due to overflow.)
>
> Signed-off-by: Anderson Lizardo <anderson.lizardo@openbossa.org>
> ---
> net/bluetooth/hidp/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Patch has been applied to bluetooth.git. Thanks. I'm also sending it to
stable.
Gustavo
^ permalink raw reply
* usb device removed from sysfs before input children devices
From: Karl Relton @ 2013-01-09 19:27 UTC (permalink / raw)
To: linux-usb, linux-bluetooth
On coming out of suspend my usb bluetooth adaptor is being reset by the
system.
In linux 3.7 the usb devices are being removed from the sysfs tree
first, and then the various 'child' devices (like my bluetooth mouse &
keyboard related devices) afterwards. This is causing the udev events
for the input devices to have 'orphaned' sysfs paths in the udev events.
This in turn means the Xorg evdev driver does not recognise the events,
and so doesn't see the removal of the input devices.
This has been picked by some downstream distributions, e.g. see this
thread by Google Chrome developers:
http://code.google.com/p/chromium-os/issues/detail?id=33813
Back on linux 3.2 this was not the case. The usb adaptor was reset, but
device removal was orderly: first the input devices (will full paths in
the udev events), then the usb devices walking up the tree.
To illustrate the issue, here is the output of 'udevadm monitor' in 3.7:
udevadm monitor
monitor will print the received events for:
KERNEL - the kernel uevent
KERNEL[2203.173080] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/rfkill2 (rfkill)
KERNEL[2203.173148] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0 (bluetooth)
KERNEL[2203.173420] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0 (usb)
KERNEL[2203.173451] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.1 (usb)
KERNEL[2203.173475] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.2 (usb)
KERNEL[2203.173693] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2 (usb)
KERNEL[2213.152339] remove /hci0/hci0:46/input14/mouse2 (input)
KERNEL[2213.160374] remove /hci0/hci0:46/input14/event10 (input)
KERNEL[2213.168366] remove /hci0/hci0:46/input14 (input)
KERNEL[2213.169058] remove /hci0/hci0:46/0005:050D:0031.0005/hidraw/hidraw0 (hidraw)
KERNEL[2213.169198] remove /hci0/hci0:46/0005:050D:0031.0005 (hid)
KERNEL[2213.169242] remove /hci0/hci0:46 (bluetooth)
KERNEL[2218.176527] remove /hci0/hci0:49/input13/event11 (input)
KERNEL[2218.180403] remove /hci0/hci0:49/input13 (input)
KERNEL[2218.180481] remove /hci0/hci0:49/0005:05AC:0256.0004/hidraw/hidraw1 (hidraw)
KERNEL[2218.180538] remove /hci0/hci0:49/0005:05AC:0256.0004 (hid)
KERNEL[2218.182005] remove /hci0/hci0:49 (bluetooth)
See how the usb devices are moved first, and then the input/bluetooth related stuff
with path-heads removed (paths are now /hci0/... instead of /devices/...)
Here is the equiv sequence back in 3.2:
KERNEL[158.378301] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:49/input11/mouse2 (input)
KERNEL[158.388283] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:49/input11/event11 (input)
KERNEL[158.409885] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:49/input11 (input)
KERNEL[158.411565] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:49/0005:050D:0031.0002/hidraw/hidraw1 (hidraw)
KERNEL[158.411598] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:49/0005:050D:0031.0002 (hid)
KERNEL[158.411621] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:49 (bluetooth)
KERNEL[158.436894] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:46/input10/event10 (input)
KERNEL[158.452211] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:46/input10 (input)
KERNEL[158.452628] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:46/0005:05AC:0256.0001/hidraw/hidraw0 (hidraw)
KERNEL[158.452662] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:46/0005:05AC:0256.0001 (hid)
KERNEL[158.452752] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:46 (bluetooth)
KERNEL[158.629847] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/rfkill2 (rfkill)
KERNEL[158.629920] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0 (bluetooth)
KERNEL[158.635562] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0 (usb)
KERNEL[158.635701] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.1 (usb)
KERNEL[158.635807] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.2 (usb)
KERNEL[158.637238] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2 (usb)
The end result (for the user) is that even when the bluetooth
mouse/keyboard is re-added, Xorg ignores it - thinking it is some hoax
duplicate device. The keyboard/mouse is then non-operational.
Karl
^ permalink raw reply
* Re: [PATCH BlueZ 2/3] unit: Rename x_pdu() macro on SDP test program
From: Marcel Holtmann @ 2013-01-09 19:10 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1357744762-5111-2-git-send-email-anderson.lizardo@openbossa.org>
Hi Anderson,
> Using the "raw_data" name makes sense, given the macro is just casting
> input (raw) data. It will also be reused in other tests with raw input
> data.
>
> Also fix this minor checkpatch.pl error:
>
> ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in
> parenthesis
> +#define raw_data(args...) (const unsigned char[]) { args }
> ---
> unit/test-sdp.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
patch has been applied.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH BlueZ 1/3] unit: Reuse define_test() macro for /TP/SERVER/BRW/* tests
From: Marcel Holtmann @ 2013-01-09 19:08 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1357744762-5111-1-git-send-email-anderson.lizardo@openbossa.org>
Hi Anderson,
> This is made possible by adding the mtu parameter, given
> /TP/SERVER/BRW/* tests use MTU of 672.
> ---
> unit/test-sdp.c | 24 ++++++------------------
> 1 file changed, 6 insertions(+), 18 deletions(-)
>
> diff --git a/unit/test-sdp.c b/unit/test-sdp.c
> index 315a5cd..e9cbcdf 100644
> --- a/unit/test-sdp.c
> +++ b/unit/test-sdp.c
> @@ -68,34 +68,22 @@ struct test_data {
> .cont_len = cont, \
> }
>
> -#define define_test(name, args...) \
> +#define define_test(name, _mtu, args...) \
> do { \
> const struct sdp_pdu pdus[] = { \
> args, { }, { } \
> }; \
> static struct test_data data; \
> - data.mtu = 48; \
> + data.mtu = _mtu; \
so using _mtu instead of mtu was the trick here. For the heck of it I
could not figure that one out. So in the end I gave up on it and
hardcoded it :(
Patch has been applied.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v1] Bluetooth: Fix authentication if acl data comes before remote feature evt
From: Mikel Astiz @ 2013-01-09 16:27 UTC (permalink / raw)
To: Jaganath Kanakkassery; +Cc: linux-bluetooth
In-Reply-To: <1357543751-12204-1-git-send-email-jaganath.k@samsung.com>
Hi Jaganath,
On Mon, Jan 7, 2013 at 8:29 AM, Jaganath Kanakkassery
<jaganath.k@samsung.com> wrote:
> If remote device sends l2cap info request before read_remote_ext_feature
> completes then mgmt_connected will be sent in hci_acldata_packet() and
> remote name request wont be sent and eventually authentication wont happen
>
> Hcidump log of the issue
>
> < HCI Command: Create Connection (0x01|0x0005) plen 13
> bdaddr BC:85:1F:74:7F:29 ptype 0xcc18 rswitch 0x01 clkoffset 0x4bf7 (valid)
> Packet type: DM1 DM3 DM5 DH1 DH3 DH5
>> HCI Event: Command Status (0x0f) plen 4
> Create Connection (0x01|0x0005) status 0x00 ncmd 1
>> HCI Event: Connect Complete (0x03) plen 11
> status 0x00 handle 12 bdaddr BC:85:1F:74:7F:29 type ACL encrypt 0x00
> < HCI Command: Read Remote Supported Features (0x01|0x001b) plen 2
> handle 12
>> HCI Event: Command Status (0x0f) plen 4
> Read Remote Supported Features (0x01|0x001b) status 0x00 ncmd 1
>> HCI Event: Read Remote Supported Features (0x0b) plen 11
> status 0x00 handle 12
> Features: 0xbf 0xfe 0xcf 0xfe 0xdb 0xff 0x7b 0x87
>> HCI Event: Max Slots Change (0x1b) plen 3
> handle 12 slots 5
> < HCI Command: Read Remote Extended Features (0x01|0x001c) plen 3
> handle 12 page 1
>> HCI Event: Command Status (0x0f) plen 4
> Read Remote Extended Features (0x01|0x001c) status 0x00 ncmd 1
>> ACL data: handle 12 flags 0x02 dlen 10
> L2CAP(s): Info req: type 2
> < ACL data: handle 12 flags 0x00 dlen 16
> L2CAP(s): Info rsp: type 2 result 0
> Extended feature mask 0x00b8
> Enhanced Retransmission mode
> Streaming mode
> FCS Option
> Fixed Channels
>> HCI Event: Read Remote Extended Features (0x23) plen 13
> status 0x00 handle 12 page 1 max 1
> Features: 0x01 0x00 0x00 0x00 0x00 0x00 0x00 0x00
>> ACL data: handle 12 flags 0x02 dlen 10
> L2CAP(s): Info req: type 3
> < ACL data: handle 12 flags 0x00 dlen 20
> L2CAP(s): Info rsp: type 3 result 0
> Fixed channel list 0x00000002
> L2CAP Signalling Channel
>> HCI Event: Number of Completed Packets (0x13) plen 5
> handle 12 packets 2
>
> This patch moves sending mgmt_connected from hci_acldata_packet() to
> l2cap_connect_req() since this code is to handle the scenario remote
> device sends l2cap connect req too fast
> ---
> v1 ---> Incorporated Johan's comments - Instead of fixing in hci_acldata_packet(),
> move sending mgmt_connected to l2cap_connect_req since this code is mainly to
> handle the scenario if remote device sends l2cap connection too fast
>
> net/bluetooth/hci_core.c | 8 --------
> net/bluetooth/l2cap_core.c | 11 +++++++++++
> 2 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 596660d..0f78e34 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2810,14 +2810,6 @@ static void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)
> if (conn) {
> hci_conn_enter_active_mode(conn, BT_POWER_FORCE_ACTIVE_OFF);
>
> - hci_dev_lock(hdev);
> - if (test_bit(HCI_MGMT, &hdev->dev_flags) &&
> - !test_and_set_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags))
> - mgmt_device_connected(hdev, &conn->dst, conn->type,
> - conn->dst_type, 0, NULL, 0,
> - conn->dev_class);
> - hci_dev_unlock(hdev);
> -
> /* Send to upper protocol */
> l2cap_recv_acldata(conn, skb, flags);
> return;
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 82a3bdc..7c7e932 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -3722,6 +3722,17 @@ sendresp:
> static int l2cap_connect_req(struct l2cap_conn *conn,
> struct l2cap_cmd_hdr *cmd, u8 *data)
> {
> + struct hci_dev *hdev = conn->hcon->hdev;
> + struct hci_conn *hcon = conn->hcon;
> +
> + hci_dev_lock(hdev);
> + if (test_bit(HCI_MGMT, &hdev->dev_flags) &&
> + !test_and_set_bit(HCI_CONN_MGMT_CONNECTED, &hcon->flags))
> + mgmt_device_connected(hdev, &hcon->dst, hcon->type,
> + hcon->dst_type, 0, NULL, 0,
> + hcon->dev_class);
> + hci_dev_unlock(hdev);
> +
> l2cap_connect(conn, cmd, data, L2CAP_CONN_RSP, 0);
> return 0;
> }
I've tested this patch and it seems to solve another issue: during an
incoming pairing procedure with a phone, the name of the device could
be left unresolved.
The issue was 100% reproducible in my setup with certain phones, for
example iPhone 5, and it gets fixed with this patch.
Cheers,
Mikel
^ permalink raw reply
* [PATCH v2 BlueZ 3/3] unit: Implement tests for sdp_extract_attr()
From: Anderson Lizardo @ 2013-01-09 15:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1357744762-5111-1-git-send-email-anderson.lizardo@openbossa.org>
These tests are for valid data. Other tests might be added later to
check for error paths, but will require separate macros.
As example of failure output, if commit
504a0cf46ad89cab8005ce9cffb22e41048f6a30 is reverted for testing, the
STR16 test will fail with:
ERROR:unit/test-sdp.c:776:test_sdp_de_attr: assertion failed
(test->input_size == size): (7 == 11)
---
unit/test-sdp.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 65 insertions(+)
diff --git a/unit/test-sdp.c b/unit/test-sdp.c
index 77a4c6c..d511ef4 100644
--- a/unit/test-sdp.c
+++ b/unit/test-sdp.c
@@ -85,6 +85,29 @@ struct test_data {
#define define_ssa(name, args...) define_test("/TP/SERVER/SSA/" name, 48, args)
#define define_brw(name, args...) define_test("/TP/SERVER/BRW/" name, 672, args)
+/* SDP Data Element (DE) tests */
+struct test_data_de {
+ const void *input_data;
+ size_t input_size;
+ sdp_data_t expected;
+};
+
+#define exp_data(_dtd, val_type, val_data) \
+ ((const sdp_data_t) { \
+ .dtd = _dtd, \
+ .val.val_type = val_data, \
+ })
+
+#define define_test_de_attr(name, input, exp) \
+ do { \
+ static struct test_data_de data; \
+ data.input_data = input; \
+ data.input_size = sizeof(input); \
+ data.expected = exp; \
+ g_test_add_data_func("/sdp/DE/ATTR/" name, &data, \
+ test_sdp_de_attr); \
+ } while (0)
+
struct context {
GMainLoop *main_loop;
guint server_source;
@@ -742,6 +765,35 @@ static void test_sdp(gconstpointer data)
g_free(test->pdu_list);
}
+static void test_sdp_de_attr(gconstpointer data)
+{
+ const struct test_data_de *test = data;
+ sdp_data_t *d;
+ int size = 0;
+
+ d = sdp_extract_attr(test->input_data, test->input_size, &size, NULL);
+ g_assert(d != NULL);
+ g_assert_cmpuint(test->input_size, ==, size);
+ g_assert_cmpuint(test->expected.dtd, ==, d->dtd);
+
+ if (g_test_verbose() == TRUE)
+ g_print("DTD=0x%02x\n", d->dtd);
+
+ switch (d->dtd) {
+ case SDP_TEXT_STR8:
+ case SDP_TEXT_STR16:
+ case SDP_URL_STR8:
+ case SDP_URL_STR16:
+ g_assert_cmpstr(test->expected.val.str, ==, d->val.str);
+ break;
+ /* TODO: implement other DTDs here */
+ default:
+ g_assert_not_reached();
+ }
+
+ sdp_data_free(d);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -2697,5 +2749,18 @@ int main(int argc, char *argv[])
0x08, 0x09, 0x00, 0x01, 0x35, 0x03, 0x19, 0x11,
0x06, 0x00));
+ /*
+ * SDP Data Element (DE) tests
+ *
+ * Test extraction of valid DEs supported by sdp_extract_attr().
+ */
+ define_test_de_attr("STR8",
+ raw_data(0x25, 0x04, 0x41, 0x42, 0x43, 0x44),
+ exp_data(SDP_TEXT_STR8, str, "ABCD"));
+ define_test_de_attr("STR16",
+ raw_data(0x26, 0x00, 0x04, 0x41, 0x42, 0x43, 0x44),
+ exp_data(SDP_TEXT_STR16, str, "ABCD"));
+ /* TODO: implement other tests for valid DEs */
+
return g_test_run();
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH BlueZ 2/3] unit: Rename x_pdu() macro on SDP test program
From: Anderson Lizardo @ 2013-01-09 15:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1357744762-5111-1-git-send-email-anderson.lizardo@openbossa.org>
Using the "raw_data" name makes sense, given the macro is just casting
input (raw) data. It will also be reused in other tests with raw input
data.
Also fix this minor checkpatch.pl error:
ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in
parenthesis
+#define raw_data(args...) (const unsigned char[]) { args }
---
unit/test-sdp.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/unit/test-sdp.c b/unit/test-sdp.c
index e9cbcdf..77a4c6c 100644
--- a/unit/test-sdp.c
+++ b/unit/test-sdp.c
@@ -51,20 +51,20 @@ struct test_data {
struct sdp_pdu *pdu_list;
};
-#define x_pdu(args...) (const unsigned char[]) { args }
+#define raw_data(args...) ((const unsigned char[]) { args })
#define raw_pdu(args...) \
{ \
.valid = true, \
- .raw_data = x_pdu(args), \
- .raw_size = sizeof(x_pdu(args)), \
+ .raw_data = raw_data(args), \
+ .raw_size = sizeof(raw_data(args)), \
}
#define raw_pdu_cont(cont, args...) \
{ \
.valid = true, \
- .raw_data = x_pdu(args), \
- .raw_size = sizeof(x_pdu(args)), \
+ .raw_data = raw_data(args), \
+ .raw_size = sizeof(raw_data(args)), \
.cont_len = cont, \
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH BlueZ 1/3] unit: Reuse define_test() macro for /TP/SERVER/BRW/* tests
From: Anderson Lizardo @ 2013-01-09 15:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
This is made possible by adding the mtu parameter, given
/TP/SERVER/BRW/* tests use MTU of 672.
---
unit/test-sdp.c | 24 ++++++------------------
1 file changed, 6 insertions(+), 18 deletions(-)
diff --git a/unit/test-sdp.c b/unit/test-sdp.c
index 315a5cd..e9cbcdf 100644
--- a/unit/test-sdp.c
+++ b/unit/test-sdp.c
@@ -68,34 +68,22 @@ struct test_data {
.cont_len = cont, \
}
-#define define_test(name, args...) \
+#define define_test(name, _mtu, args...) \
do { \
const struct sdp_pdu pdus[] = { \
args, { }, { } \
}; \
static struct test_data data; \
- data.mtu = 48; \
+ data.mtu = _mtu; \
data.pdu_list = g_malloc(sizeof(pdus)); \
memcpy(data.pdu_list, pdus, sizeof(pdus)); \
g_test_add_data_func(name, &data, test_sdp); \
} while (0)
-#define define_ss(name, args...) define_test("/TP/SERVER/SS/" name, args)
-#define define_sa(name, args...) define_test("/TP/SERVER/SA/" name, args)
-#define define_ssa(name, args...) define_test("/TP/SERVER/SSA/" name, args)
-
-#define define_brw(name, args...) \
- do { \
- const struct sdp_pdu pdus[] = { \
- args, { }, { } \
- }; \
- static struct test_data data; \
- data.mtu = 672; \
- data.pdu_list = g_malloc(sizeof(pdus)); \
- memcpy(data.pdu_list, pdus, sizeof(pdus)); \
- g_test_add_data_func("/TP/SERVER/BRW/" name, \
- &data, test_sdp); \
- } while (0)
+#define define_ss(name, args...) define_test("/TP/SERVER/SS/" name, 48, args)
+#define define_sa(name, args...) define_test("/TP/SERVER/SA/" name, 48, args)
+#define define_ssa(name, args...) define_test("/TP/SERVER/SSA/" name, 48, args)
+#define define_brw(name, args...) define_test("/TP/SERVER/BRW/" name, 672, args)
struct context {
GMainLoop *main_loop;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH BlueZ 1/2] unit: Add initial tests for sdp_extract_attr()
From: Anderson Lizardo @ 2013-01-09 15:14 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1357685187.1806.40.camel@aeonflux>
Hi Marcel,
On Tue, Jan 8, 2013 at 6:46 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> A better test case data could look like this:
>
> struct test_data {
> const void *input_data;
> size_t input_size;
> uint8_t dtd;
> };
>
> And we can add combinations of structs and unions here to represent the
> different testes.
>
> Also I would create a nice helper define like I did with define_ssa()
> for example to create the test data and the test with a nice name.
I just sent a v2 of the SDP DE tests based on your suggestions (the 2
other patches are just refactoring of existing code).
Let me know if this new format is fine, so I can proceed with adding
other tests.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* [PATCH 7/8 v2] Bluetooth: Fix checking for exact values of boolean mgmt parameters
From: Johan Hedberg @ 2013-01-09 14:05 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1357738180-4128-8-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
All mgmt_set_* commands that take a boolean value encoded in the form of
a byte should only accept the values 0x00 and 0x01. This patch adds the
necessary checks for this and returns "invalid params" responses if
anything else is provided as the value.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
v2: Fix s/SET_SSP/SET_LE/ copy-paste issue
net/bluetooth/mgmt.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 69221ce..3cf7e1d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -777,6 +777,10 @@ static int set_powered(struct sock *sk, struct hci_dev *hdev, void *data,
BT_DBG("request for %s", hdev->name);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_POWERED,
+ MGMT_STATUS_INVALID_PARAMS);
+
hci_dev_lock(hdev);
if (test_and_clear_bit(HCI_AUTO_OFF, &hdev->dev_flags)) {
@@ -872,6 +876,10 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
MGMT_STATUS_NOT_SUPPORTED);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
+ MGMT_STATUS_INVALID_PARAMS);
+
timeout = __le16_to_cpu(cp->timeout);
if (!cp->val && timeout > 0)
return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
@@ -971,6 +979,10 @@ static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
return cmd_status(sk, hdev->id, MGMT_OP_SET_CONNECTABLE,
MGMT_STATUS_NOT_SUPPORTED);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_CONNECTABLE,
+ MGMT_STATUS_INVALID_PARAMS);
+
hci_dev_lock(hdev);
if (!hdev_is_powered(hdev)) {
@@ -1041,6 +1053,10 @@ static int set_pairable(struct sock *sk, struct hci_dev *hdev, void *data,
BT_DBG("request for %s", hdev->name);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_PAIRABLE,
+ MGMT_STATUS_INVALID_PARAMS);
+
hci_dev_lock(hdev);
if (cp->val)
@@ -1073,6 +1089,10 @@ static int set_link_security(struct sock *sk, struct hci_dev *hdev, void *data,
return cmd_status(sk, hdev->id, MGMT_OP_SET_LINK_SECURITY,
MGMT_STATUS_NOT_SUPPORTED);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_LINK_SECURITY,
+ MGMT_STATUS_INVALID_PARAMS);
+
hci_dev_lock(hdev);
if (!hdev_is_powered(hdev)) {
@@ -1137,6 +1157,10 @@ static int set_ssp(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
return cmd_status(sk, hdev->id, MGMT_OP_SET_SSP,
MGMT_STATUS_NOT_SUPPORTED);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_SSP,
+ MGMT_STATUS_INVALID_PARAMS);
+
hci_dev_lock(hdev);
val = !!cp->val;
@@ -1197,6 +1221,10 @@ static int set_hs(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
return cmd_status(sk, hdev->id, MGMT_OP_SET_HS,
MGMT_STATUS_NOT_SUPPORTED);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_HS,
+ MGMT_STATUS_INVALID_PARAMS);
+
if (cp->val)
set_bit(HCI_HS_ENABLED, &hdev->dev_flags);
else
@@ -1219,6 +1247,10 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
return cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
MGMT_STATUS_NOT_SUPPORTED);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
+ MGMT_STATUS_INVALID_PARAMS);
+
hci_dev_lock(hdev);
val = !!cp->val;
@@ -2630,6 +2662,10 @@ static int set_fast_connectable(struct sock *sk, struct hci_dev *hdev,
return cmd_status(sk, hdev->id, MGMT_OP_SET_FAST_CONNECTABLE,
MGMT_STATUS_NOT_SUPPORTED);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_FAST_CONNECTABLE,
+ MGMT_STATUS_INVALID_PARAMS);
+
if (!hdev_is_powered(hdev))
return cmd_status(sk, hdev->id, MGMT_OP_SET_FAST_CONNECTABLE,
MGMT_STATUS_NOT_POWERED);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH 7/8] Bluetooth: Fix checking for exact values of boolean mgmt parameters
From: Anderson Lizardo @ 2013-01-09 13:53 UTC (permalink / raw)
To: Anderson Lizardo, linux-bluetooth
In-Reply-To: <20130109134837.GA4933@x220>
Hi Johan,
On Wed, Jan 9, 2013 at 9:48 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> Can mgmt-tester be modified to check for this as well?
>
> Yes, I don't think it has any set_le test cases yet. Patches are
> welcome!
Unless someone else takes care of this, I can check after finishing
the SDP unit tests I was working on.
>
> Johan
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH 7/8] Bluetooth: Fix checking for exact values of boolean mgmt parameters
From: Johan Hedberg @ 2013-01-09 13:48 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <CAJdJm_PZ5i-3HqmbL7YT5F59ycBJP3g83RMcWBBmKS9H02NJbQ@mail.gmail.com>
Hi Lizardo,
On Wed, Jan 09, 2013, Anderson Lizardo wrote:
> Hi Johan,
>
> On Wed, Jan 9, 2013 at 9:29 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> > @@ -1219,6 +1247,10 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
> > return cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
> > MGMT_STATUS_NOT_SUPPORTED);
> >
> > + if (cp->val != 0x00 && cp->val != 0x01)
> > + return cmd_status(sk, hdev->id, MGMT_OP_SET_SSP,
> > + MGMT_STATUS_INVALID_PARAMS);
> > +
>
> I think you meant "MGMT_OP_SET_LE" here.
Good catch. A copy-paste mistake I should have caught.
> Can mgmt-tester be modified to check for this as well?
Yes, I don't think it has any set_le test cases yet. Patches are
welcome!
Johan
^ permalink raw reply
* Re: [PATCH 7/8] Bluetooth: Fix checking for exact values of boolean mgmt parameters
From: Anderson Lizardo @ 2013-01-09 13:45 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1357738180-4128-8-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
On Wed, Jan 9, 2013 at 9:29 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> @@ -1219,6 +1247,10 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
> return cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
> MGMT_STATUS_NOT_SUPPORTED);
>
> + if (cp->val != 0x00 && cp->val != 0x01)
> + return cmd_status(sk, hdev->id, MGMT_OP_SET_SSP,
> + MGMT_STATUS_INVALID_PARAMS);
> +
I think you meant "MGMT_OP_SET_LE" here.
Can mgmt-tester be modified to check for this as well?
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* [PATCH 8/8] Bluetooth: Fix sending incorrect new_settings for mgmt_set_powered
From: Johan Hedberg @ 2013-01-09 13:29 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1357738180-4128-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
The socket from which a mgmt_set_powered command was received should
only receive the command response but no new_settings event.
The mgmt_powered() function which is used to handle the situation with
the HCI_AUTO_OFF flag tries to check for a pending command to know which
socket to skip the event for, but since the pending command hasn't been
added this will not happen.
This patch fixes the issue by adding the pending command for the
HCI_AUTO_OFF case and thereby ensures that mgmt_powered() will skip the
right socket when sending the new_settings event, but still send the
proper response to the socket where the command came from.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index eb9d48a..60afaee 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -787,8 +787,9 @@ static int set_powered(struct sock *sk, struct hci_dev *hdev, void *data,
cancel_delayed_work(&hdev->power_off);
if (cp->val) {
- err = send_settings_rsp(sk, MGMT_OP_SET_POWERED, hdev);
- mgmt_powered(hdev, 1);
+ mgmt_pending_add(sk, MGMT_OP_SET_POWERED, hdev,
+ data, len);
+ err = mgmt_powered(hdev, 1);
goto failed;
}
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 7/8] Bluetooth: Fix checking for exact values of boolean mgmt parameters
From: Johan Hedberg @ 2013-01-09 13:29 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1357738180-4128-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
All mgmt_set_* commands that take a boolean value encoded in the form of
a byte should only accept the values 0x00 and 0x01. This patch adds the
necessary checks for this and returns "invalid params" responses if
anything else is provided as the value.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 69221ce..eb9d48a 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -777,6 +777,10 @@ static int set_powered(struct sock *sk, struct hci_dev *hdev, void *data,
BT_DBG("request for %s", hdev->name);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_POWERED,
+ MGMT_STATUS_INVALID_PARAMS);
+
hci_dev_lock(hdev);
if (test_and_clear_bit(HCI_AUTO_OFF, &hdev->dev_flags)) {
@@ -872,6 +876,10 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
MGMT_STATUS_NOT_SUPPORTED);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
+ MGMT_STATUS_INVALID_PARAMS);
+
timeout = __le16_to_cpu(cp->timeout);
if (!cp->val && timeout > 0)
return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
@@ -971,6 +979,10 @@ static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
return cmd_status(sk, hdev->id, MGMT_OP_SET_CONNECTABLE,
MGMT_STATUS_NOT_SUPPORTED);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_CONNECTABLE,
+ MGMT_STATUS_INVALID_PARAMS);
+
hci_dev_lock(hdev);
if (!hdev_is_powered(hdev)) {
@@ -1041,6 +1053,10 @@ static int set_pairable(struct sock *sk, struct hci_dev *hdev, void *data,
BT_DBG("request for %s", hdev->name);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_PAIRABLE,
+ MGMT_STATUS_INVALID_PARAMS);
+
hci_dev_lock(hdev);
if (cp->val)
@@ -1073,6 +1089,10 @@ static int set_link_security(struct sock *sk, struct hci_dev *hdev, void *data,
return cmd_status(sk, hdev->id, MGMT_OP_SET_LINK_SECURITY,
MGMT_STATUS_NOT_SUPPORTED);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_LINK_SECURITY,
+ MGMT_STATUS_INVALID_PARAMS);
+
hci_dev_lock(hdev);
if (!hdev_is_powered(hdev)) {
@@ -1137,6 +1157,10 @@ static int set_ssp(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
return cmd_status(sk, hdev->id, MGMT_OP_SET_SSP,
MGMT_STATUS_NOT_SUPPORTED);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_SSP,
+ MGMT_STATUS_INVALID_PARAMS);
+
hci_dev_lock(hdev);
val = !!cp->val;
@@ -1197,6 +1221,10 @@ static int set_hs(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
return cmd_status(sk, hdev->id, MGMT_OP_SET_HS,
MGMT_STATUS_NOT_SUPPORTED);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_HS,
+ MGMT_STATUS_INVALID_PARAMS);
+
if (cp->val)
set_bit(HCI_HS_ENABLED, &hdev->dev_flags);
else
@@ -1219,6 +1247,10 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
return cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
MGMT_STATUS_NOT_SUPPORTED);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_SSP,
+ MGMT_STATUS_INVALID_PARAMS);
+
hci_dev_lock(hdev);
val = !!cp->val;
@@ -2630,6 +2662,10 @@ static int set_fast_connectable(struct sock *sk, struct hci_dev *hdev,
return cmd_status(sk, hdev->id, MGMT_OP_SET_FAST_CONNECTABLE,
MGMT_STATUS_NOT_SUPPORTED);
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_FAST_CONNECTABLE,
+ MGMT_STATUS_INVALID_PARAMS);
+
if (!hdev_is_powered(hdev))
return cmd_status(sk, hdev->id, MGMT_OP_SET_FAST_CONNECTABLE,
MGMT_STATUS_NOT_POWERED);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 6/8] Bluetooth: Move non-critical sections outside of the dev lock
From: Johan Hedberg @ 2013-01-09 13:29 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1357738180-4128-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
This patch fixes sections of code that do not need hci_lock_dev to be
outside of the lock. Such sections include code that do not touch the
hdev at all as well as sections which just read a single byte from the
supported_features value (i.e. all lmp_*_capable() macros).
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 46 ++++++++++++++++++----------------------------
1 file changed, 18 insertions(+), 28 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 4f60540..69221ce 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1133,13 +1133,11 @@ static int set_ssp(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
BT_DBG("request for %s", hdev->name);
- hci_dev_lock(hdev);
+ if (!lmp_ssp_capable(hdev))
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_SSP,
+ MGMT_STATUS_NOT_SUPPORTED);
- if (!lmp_ssp_capable(hdev)) {
- err = cmd_status(sk, hdev->id, MGMT_OP_SET_SSP,
- MGMT_STATUS_NOT_SUPPORTED);
- goto failed;
- }
+ hci_dev_lock(hdev);
val = !!cp->val;
@@ -1217,13 +1215,11 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
BT_DBG("request for %s", hdev->name);
- hci_dev_lock(hdev);
+ if (!lmp_le_capable(hdev))
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
+ MGMT_STATUS_NOT_SUPPORTED);
- if (!lmp_le_capable(hdev)) {
- err = cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
- MGMT_STATUS_NOT_SUPPORTED);
- goto unlock;
- }
+ hci_dev_lock(hdev);
val = !!cp->val;
enabled = lmp_host_le_capable(hdev);
@@ -1422,25 +1418,19 @@ static int set_dev_class(struct sock *sk, struct hci_dev *hdev, void *data,
BT_DBG("request for %s", hdev->name);
- hci_dev_lock(hdev);
+ if (!lmp_bredr_capable(hdev))
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_DEV_CLASS,
+ MGMT_STATUS_NOT_SUPPORTED);
- if (!lmp_bredr_capable(hdev)) {
- err = cmd_status(sk, hdev->id, MGMT_OP_SET_DEV_CLASS,
- MGMT_STATUS_NOT_SUPPORTED);
- goto unlock;
- }
+ if (test_bit(HCI_PENDING_CLASS, &hdev->dev_flags))
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_DEV_CLASS,
+ MGMT_STATUS_BUSY);
- if (test_bit(HCI_PENDING_CLASS, &hdev->dev_flags)) {
- err = cmd_status(sk, hdev->id, MGMT_OP_SET_DEV_CLASS,
- MGMT_STATUS_BUSY);
- goto unlock;
- }
+ if ((cp->minor & 0x03) != 0 || (cp->major & 0xe0) != 0)
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_DEV_CLASS,
+ MGMT_STATUS_INVALID_PARAMS);
- if ((cp->minor & 0x03) != 0 || (cp->major & 0xe0) != 0) {
- err = cmd_status(sk, hdev->id, MGMT_OP_SET_DEV_CLASS,
- MGMT_STATUS_INVALID_PARAMS);
- goto unlock;
- }
+ hci_dev_lock(hdev);
hdev->major_class = cp->major;
hdev->minor_class = cp->minor;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 5/8] Bluetooth: Fix returning proper command status for start_discovery
From: Johan Hedberg @ 2013-01-09 13:29 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1357738180-4128-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
Management commands should whenever possible fail with proper command
status or command complete events. This patch fixes the
mgmt_start_discovery command to do this for the failure cases where an
incorrect parameter value was passed to it ("not supported" if the
parameter value was valid but the controller doesn't support it and
"invalid params" if it isn't valid at all).
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 35 +++++++++++++++++++++++++----------
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 6e6de9e..4f60540 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2377,31 +2377,46 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
switch (hdev->discovery.type) {
case DISCOV_TYPE_BREDR:
- if (lmp_bredr_capable(hdev))
+ if (lmp_bredr_capable(hdev)) {
err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
- else
- err = -ENOTSUPP;
+ } else {
+ err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY,
+ MGMT_STATUS_NOT_SUPPORTED);
+ mgmt_pending_remove(cmd);
+ goto failed;
+ }
break;
case DISCOV_TYPE_LE:
- if (lmp_host_le_capable(hdev))
+ if (lmp_host_le_capable(hdev)) {
err = hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT,
LE_SCAN_WIN, LE_SCAN_TIMEOUT_LE_ONLY);
- else
- err = -ENOTSUPP;
+ } else {
+ err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY,
+ MGMT_STATUS_NOT_SUPPORTED);
+ mgmt_pending_remove(cmd);
+ goto failed;
+ }
break;
case DISCOV_TYPE_INTERLEAVED:
- if (lmp_host_le_capable(hdev) && lmp_bredr_capable(hdev))
+ if (lmp_host_le_capable(hdev) && lmp_bredr_capable(hdev)) {
err = hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT,
LE_SCAN_WIN,
LE_SCAN_TIMEOUT_BREDR_LE);
- else
- err = -ENOTSUPP;
+ } else {
+ err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY,
+ MGMT_STATUS_NOT_SUPPORTED);
+ mgmt_pending_remove(cmd);
+ goto failed;
+ }
break;
default:
- err = -EINVAL;
+ err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY,
+ MGMT_STATUS_INVALID_PARAMS);
+ mgmt_pending_remove(cmd);
+ goto failed;
}
if (err < 0)
--
1.7.10.4
^ permalink raw reply related
* [PATCH 4/8] Bluetooth: Fix accepting set_dev_class for non-BR/EDR controllers
From: Johan Hedberg @ 2013-01-09 13:29 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1357738180-4128-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
The concept of Class of Device only exists for BR/EDR controllers. The
mgmt_set_dev_class command should therefore return a proper "not
supported" error if it is attempted for a controller that doesn't
support BR/EDR (e.g. a single mode LE-only one).
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 2785de2..6e6de9e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1424,6 +1424,12 @@ static int set_dev_class(struct sock *sk, struct hci_dev *hdev, void *data,
hci_dev_lock(hdev);
+ if (!lmp_bredr_capable(hdev)) {
+ err = cmd_status(sk, hdev->id, MGMT_OP_SET_DEV_CLASS,
+ MGMT_STATUS_NOT_SUPPORTED);
+ goto unlock;
+ }
+
if (test_bit(HCI_PENDING_CLASS, &hdev->dev_flags)) {
err = cmd_status(sk, hdev->id, MGMT_OP_SET_DEV_CLASS,
MGMT_STATUS_BUSY);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 3/8] Bluetooth: Fix checking for valid device class values
From: Johan Hedberg @ 2013-01-09 13:29 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1357738180-4128-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
The two lowest bits of the minor device class value are reserved and
should be zero, and the three highest bits of the major device class
likewise. The management code should therefore test for this and return
a proper "invalid params" error if the condition is not met.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index aaf9ce6..2785de2 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1430,6 +1430,12 @@ static int set_dev_class(struct sock *sk, struct hci_dev *hdev, void *data,
goto unlock;
}
+ if ((cp->minor & 0x03) != 0 || (cp->major & 0xe0) != 0) {
+ err = cmd_status(sk, hdev->id, MGMT_OP_SET_DEV_CLASS,
+ MGMT_STATUS_INVALID_PARAMS);
+ goto unlock;
+ }
+
hdev->major_class = cp->major;
hdev->minor_class = cp->minor;
--
1.7.10.4
^ permalink raw reply related
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