* Re: [PATCH 1/5] Bluetooth: Only enable for L2CAP FCS for ERTM or streaming
From: Gustavo F. Padovan @ 2010-08-25 17:11 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, marcel, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1282689346-20371-2-git-send-email-mathewm@codeaurora.org>
Hi Mat,
* Mat Martineau <mathewm@codeaurora.org> [2010-08-24 15:35:42 -0700]:
> This fixes a bug which caused the FCS setting to show L2CAP_FCS_CRC16
> with L2CAP modes other than ERTM or streaming. At present, this only
> affects the FCS value shown with getsockopt() for basic mode.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap.c | 19 +++++++++++++------
> 1 files changed, 13 insertions(+), 6 deletions(-)
This one is on bluetooth-testing, but Marcel should move it to
bluetooth-2.6 as this patch is planned to go into 2.6.36
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* [PATCH 4/4] Bluetooth: Send Invalid Syntax Error if Resp Size Less Than 0x07
From: Angela Bartholomaus @ 2010-08-25 17:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Angela Bartholomaus
In-Reply-To: <1282755705-28400-1-git-send-email-angelab@codeaurora.org>
Byte cnt range min 0x07 per Core v2.1, sec 4.61 for TP/SERVER/SA/BI-02-C
---
src/sdpd-request.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/src/sdpd-request.c b/src/sdpd-request.c
index 8547939..205b27b 100644
--- a/src/sdpd-request.c
+++ b/src/sdpd-request.c
@@ -684,6 +684,15 @@ static int service_attr_req(sdp_req_t *req, sdp_buf_t *buf)
SDPDBG("max_rsp_size : %d", max_rsp_size);
/*
+ * Check that max_rsp_size is within valid range
+ * a minimum size of 0x0007 has to be used for data field
+ */
+ if (max_rsp_size < 0x0007) {
+ status = SDP_INVALID_SYNTAX;
+ goto done;
+ }
+
+ /*
* Calculate Attribute size acording to MTU
* We can send only (MTU - sizeof(sdp_pdu_hdr_t) - sizeof(sdp_cont_state_t))
*/
--
1.7.2.2
--
Angela Bartholomaus
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH 3/4] Bluetooth: Send an Invalid PDU Size Error Resp for Service Attr Search Req
From: Angela Bartholomaus @ 2010-08-25 17:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Angela Bartholomaus
In-Reply-To: <1282755705-28400-1-git-send-email-angelab@codeaurora.org>
Send error code 0x04 per CoreSpecv2.1, sec 4.4 for TP/SERVER/SSA/BI-02-C
---
src/sdpd-request.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/sdpd-request.c b/src/sdpd-request.c
index ccbd920..8547939 100644
--- a/src/sdpd-request.c
+++ b/src/sdpd-request.c
@@ -818,7 +818,7 @@ static int service_search_attr_req(sdp_req_t *req, sdp_buf_t *buf)
plen = ntohs(((sdp_pdu_hdr_t *)(req->buf))->plen);
if (plen < totscanned || plen != totscanned + *(uint8_t *)pdata) {
- status = SDP_INVALID_SYNTAX;
+ status = SDP_INVALID_PDU_SIZE;
goto done;
}
--
1.7.2.2
--
Angela Bartholomaus
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH 2/4] Bluetooth: Send an Invalid PDU Size Error Response for Service Attr Req
From: Angela Bartholomaus @ 2010-08-25 17:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Angela Bartholomaus
In-Reply-To: <1282755705-28400-1-git-send-email-angelab@codeaurora.org>
Send error code 0x04 per CoreSpecv2.1, sec 4.4 for TP/SERVER/SA/BI-03-C
---
src/sdpd-request.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/sdpd-request.c b/src/sdpd-request.c
index cc9fe82..ccbd920 100644
--- a/src/sdpd-request.c
+++ b/src/sdpd-request.c
@@ -667,7 +667,7 @@ static int service_attr_req(sdp_req_t *req, sdp_buf_t *buf)
mlen = scanned + sizeof(uint32_t) + sizeof(uint16_t) + 1;
// ensure we don't read past buffer
if (plen < mlen || plen != mlen + *(uint8_t *)pdata) {
- status = SDP_INVALID_SYNTAX;
+ status = SDP_INVALID_PDU_SIZE;
goto done;
}
--
1.7.2.2
--
Angela Bartholomaus
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH 1/4] Bluetooth: Send an Invalid PDU Size Error Response for Service Search Req
From: Angela Bartholomaus @ 2010-08-25 17:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Angela Bartholomaus
In-Reply-To: <1282755705-28400-1-git-send-email-angelab@codeaurora.org>
Send error code 0x04 per CoreSpecv2.1, sec 4.4 for TP/SERVER/SS/BI-01-C
---
src/sdpd-request.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/sdpd-request.c b/src/sdpd-request.c
index d56ffc2..cc9fe82 100644
--- a/src/sdpd-request.c
+++ b/src/sdpd-request.c
@@ -367,7 +367,7 @@ static int service_search_req(sdp_req_t *req, sdp_buf_t *buf)
mlen = scanned + sizeof(uint16_t) + 1;
// ensure we don't read past buffer
if (plen < mlen || plen != mlen + *(uint8_t *)(pdata+sizeof(uint16_t))) {
- status = SDP_INVALID_SYNTAX;
+ status = SDP_INVALID_PDU_SIZE;
goto done;
}
--
1.7.2.2
--
Angela Bartholomaus
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH v2 0/4] Fix error reporting for SDP Requests
From: Angela Bartholomaus @ 2010-08-25 17:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel
Fixed the following issues revealed while performing bluez qualification:
- Incorrect error code was being reported for requests with invalid PDU size;
- No error was being reported if a request less than 0x07 was received.
--
Angela Bartholomaus
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* Re: bluetoothd does not check remote names for valid utf8 data
From: Johan Hedberg @ 2010-08-25 15:03 UTC (permalink / raw)
To: David Vrabel; +Cc: Marcel Holtmann, linux-bluetooth
In-Reply-To: <4C75292E.9080304@csr.com>
Hi David,
On Wed, Aug 25, 2010, David Vrabel wrote:
> bluetoothd does not check in some (all?) places that the remote name
> reported by a device is valid utf8 data. e.g., extract_eir_name() in
> src/dbus-hci.c.
>
> The reception of an extended inquiry response containing a name with
> invalid utf8 data can cause the dbus interface to disappear. This is
> therefore a denial-of-service vulnerability (at the very least).
>
> The following patch fixes the above problem but there are probably other
> places where the check needs to be done.
>
> --- bluez-4.51.orig/src/dbus-hci.c
> +++ bluez-4.51/src/dbus-hci.c
> @@ -450,6 +450,8 @@
> switch (*type) {
> case 0x08:
> case 0x09:
> + if (!g_utf8_validate(data + 2, data[0] - 1, NULL))
> + return strdup("");
> return strndup((char *) (data + 2), data[0] - 1);
> }
Good catch. At least the legacy name queries are already protected
(remote_name_information function in security.c) so I think this is the only
place missing the UTF-8 validation. However, your patch doesn't compile cleanly
so some fine tuning is still needed (always check compilation with
"./bootstrap-configure && make" before sending upstream):
src/dbus-hci.c: In function ‘extract_eir_name’:
src/dbus-hci.c:466: error: pointer targets in passing argument 1 of ‘g_utf8_validate’ differ in signedness
/usr/include/glib-2.0/glib/gunicode.h:356: note: expected ‘const gchar *’ but argument is of type ‘uint8_t *’
make[1]: *** [src/dbus-hci.o] Error 1
After fixing that, could you prepare the patch through git format-patch so that
I can easily apply it using git am? Thanks.
Johan
^ permalink raw reply
* bluetoothd does not check remote names for valid utf8 data
From: David Vrabel @ 2010-08-25 14:31 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
Hi,
bluetoothd does not check in some (all?) places that the remote name
reported by a device is valid utf8 data. e.g., extract_eir_name() in
src/dbus-hci.c.
The reception of an extended inquiry response containing a name with
invalid utf8 data can cause the dbus interface to disappear. This is
therefore a denial-of-service vulnerability (at the very least).
The following patch fixes the above problem but there are probably other
places where the check needs to be done.
--- bluez-4.51.orig/src/dbus-hci.c
+++ bluez-4.51/src/dbus-hci.c
@@ -450,6 +450,8 @@
switch (*type) {
case 0x08:
case 0x09:
+ if (!g_utf8_validate(data + 2, data[0] - 1, NULL))
+ return strdup("");
return strndup((char *) (data + 2), data[0] - 1);
}
David
--
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park, Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ http://www.csr.com/
Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
^ permalink raw reply
* Re: [PATCH] No error message on interrupted syscall
From: Johan Hedberg @ 2010-08-25 13:29 UTC (permalink / raw)
To: Colin Didier; +Cc: linux-bluetooth
In-Reply-To: <20100825130749.GB7199@cybione.org>
Hi Colin,
On Wed, Aug 25, 2010, Colin Didier wrote:
> An interrupted syscall is not an error when returning from poll().
> ---
> audio/pcm_bluetooth.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
This one has also been pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] Fix leftovers handling
From: Johan Hedberg @ 2010-08-25 13:28 UTC (permalink / raw)
To: Colin Didier; +Cc: linux-bluetooth
In-Reply-To: <20100825130607.GA7199@cybione.org>
Hi Colin,
On Wed, Aug 25, 2010, Colin Didier wrote:
> If for some reason there is not enough data provided to the function
> bluetooth_a2dp_write() and there are leftovers to handle, the ALSA
> module will segfault.
> ---
> audio/pcm_bluetooth.c | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
Thanks. This patch is now upstream.
Johan
^ permalink raw reply
* [PATCH] No error message on interrupted syscall
From: Colin Didier @ 2010-08-25 13:07 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <20100825112831.GB23943@jh-x301>
An interrupted syscall is not an error when returning from poll().
---
audio/pcm_bluetooth.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/audio/pcm_bluetooth.c b/audio/pcm_bluetooth.c
index ff463fe..799f17f 100644
--- a/audio/pcm_bluetooth.c
+++ b/audio/pcm_bluetooth.c
@@ -237,9 +237,11 @@ iter_sleep:
ret = poll(fds, 2, poll_timeout);
if (ret < 0) {
- SNDERR("poll error: %s (%d)", strerror(errno), errno);
- if (errno != EINTR)
+ if (errno != EINTR) {
+ SNDERR("poll error: %s (%d)", strerror(errno),
+ errno);
break;
+ }
} else if (ret > 0) {
ret = (fds[0].revents) ? 0 : 1;
SNDERR("poll fd %d revents %d", ret, fds[ret].revents);
--
1.7.1
^ permalink raw reply related
* [PATCH] Fix leftovers handling
From: Colin Didier @ 2010-08-25 13:06 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <20100825112636.GA23943@jh-x301>
If for some reason there is not enough data provided to the function
bluetooth_a2dp_write() and there are leftovers to handle, the ALSA
module will segfault.
---
audio/pcm_bluetooth.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/audio/pcm_bluetooth.c b/audio/pcm_bluetooth.c
index 4c0ab6f..ff463fe 100644
--- a/audio/pcm_bluetooth.c
+++ b/audio/pcm_bluetooth.c
@@ -1050,8 +1050,11 @@ static snd_pcm_sframes_t bluetooth_a2dp_write(snd_pcm_ioplug_t *io,
}
/* Check if we have any left over data from the last write */
- if (data->count > 0 && (bytes_left - data->count) >= a2dp->codesize) {
- int additional_bytes_needed = a2dp->codesize - data->count;
+ if (data->count > 0) {
+ unsigned int additional_bytes_needed =
+ a2dp->codesize - data->count;
+ if (additional_bytes_needed > bytes_left)
+ goto out;
memcpy(data->buffer + data->count, buff,
additional_bytes_needed);
@@ -1122,6 +1125,7 @@ static snd_pcm_sframes_t bluetooth_a2dp_write(snd_pcm_ioplug_t *io,
}
}
+out:
/* Copy the extra to our temp buffer for the next write */
if (bytes_left > 0) {
memcpy(data->buffer + data->count, buff, bytes_left);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] An interrupted syscall is not an error when returning from poll() in audio/pcm_bluetooth.c
From: Johan Hedberg @ 2010-08-25 11:28 UTC (permalink / raw)
To: Colin Didier; +Cc: linux-bluetooth
In-Reply-To: <20100825085454.GB27072@cybione.org>
Hi Colin,
On Wed, Aug 25, 2010, Colin Didier wrote:
> An interrupted syscall is not an error when returning from poll() at
> line 240 of audio/pcm_bluetooth.c. So no need to freak out.
>
> Here is a patch to fix this.
The patch itself seems fine, but the same comments apply for the commit
message as with your other patch. I.e. prepare with git format-patch and
make sure that you don't have overly long lines (e.g. the summary line
is much too long in this one).
Johan
^ permalink raw reply
* Re: [PATCH] Fix left overs handling in bluetooth_a2dp_write() of audio/pcm_bluetooth.c
From: Johan Hedberg @ 2010-08-25 11:26 UTC (permalink / raw)
To: Colin Didier; +Cc: linux-bluetooth
In-Reply-To: <20100825084932.GA27072@cybione.org>
Hi Colin,
On Wed, Aug 25, 2010, Colin Didier wrote:
> If for some reason there is not enough data provided to the function
> bluetooth_a2dp_write() and there are left overs to handle, the ALSA
> module will segfault.
>
> The test at the line 1053 of audio/pcm_bluetooth.c is wrong because it
> compares unsigned integers and will lead to an unsigned integer overflow
> if bytes_left is inferior to a2dp->codesize - data->count.
>
> Here is a patch to fix this issue.
Thanks for the patch, however it doesn't compile cleanly:
audio/pcm_bluetooth.c: In function ‘bluetooth_a2dp_write’:
audio/pcm_bluetooth.c:1055: error: comparison between signed and unsigned integer expressions
make[1]: *** [audio/audio_libasound_module_pcm_bluetooth_la-pcm_bluetooth.lo] Error 1
make: *** [all] Error 2
Always make sure that patches compile cleanly with
./bootstrap-configure. Also, could you please use git format-patch to
create the patches and make sure they have properly formated commit
messages (fits within an 80-column terminal with git log, not formated
like an email, etc). Thanks.
Johan
^ permalink raw reply
* [PATCH] An interrupted syscall is not an error when returning from poll() in audio/pcm_bluetooth.c
From: Colin Didier @ 2010-08-25 8:54 UTC (permalink / raw)
To: linux-bluetooth
Hello,
An interrupted syscall is not an error when returning from poll() at
line 240 of audio/pcm_bluetooth.c. So no need to freak out.
Here is a patch to fix this.
Cheers,
Colin DIDIER
--- a/audio/pcm_bluetooth.c 2010-08-25 10:08:37.000000000 +0200
+++ b/audio/pcm_bluetooth.c 2010-08-25 10:08:59.000000000 +0200
@@ -237,9 +237,11 @@
ret = poll(fds, 2, poll_timeout);
if (ret < 0) {
- SNDERR("poll error: %s (%d)", strerror(errno), errno);
- if (errno != EINTR)
+ if (errno != EINTR) {
+ SNDERR("poll error: %s (%d)", strerror(errno),
+ errno);
break;
+ }
} else if (ret > 0) {
ret = (fds[0].revents) ? 0 : 1;
SNDERR("poll fd %d revents %d", ret, fds[ret].revents);
^ permalink raw reply
* [PATCH] Fix left overs handling in bluetooth_a2dp_write() of audio/pcm_bluetooth.c
From: Colin Didier @ 2010-08-25 8:49 UTC (permalink / raw)
To: linux-bluetooth
Hello,
If for some reason there is not enough data provided to the function
bluetooth_a2dp_write() and there are left overs to handle, the ALSA
module will segfault.
The test at the line 1053 of audio/pcm_bluetooth.c is wrong because it
compares unsigned integers and will lead to an unsigned integer overflow
if bytes_left is inferior to a2dp->codesize - data->count.
Here is a patch to fix this issue.
Cheers,
Colin DIDIER
--- a/audio/pcm_bluetooth.c 2010-08-25 09:35:28.000000000 +0200
+++ b/audio/pcm_bluetooth.c 2010-08-25 09:37:47.000000000 +0200
@@ -1050,8 +1050,10 @@
}
/* Check if we have any left over data from the last write */
- if (data->count > 0 && (bytes_left - data->count) >= a2dp->codesize) {
- int additional_bytes_needed = a2dp->codesize - data->count;
+ if (data->count > 0) {
+ int additional_bytes_needed = a2dp->codesize - data->count;
+ if (additional_bytes_needed > bytes_left)
+ goto out;
memcpy(data->buffer + data->count, buff,
additional_bytes_needed);
@@ -1122,6 +1124,7 @@
}
}
+out:
/* Copy the extra to our temp buffer for the next write */
if (bytes_left > 0) {
memcpy(data->buffer + data->count, buff, bytes_left);
^ permalink raw reply
* Re: [RFCOMM] Cannot complete connections into Ubuntu
From: Luiz Augusto von Dentz @ 2010-08-25 3:00 UTC (permalink / raw)
To: Shreesh Holla; +Cc: linux-bluetooth
In-Reply-To: <131634.48135.qm@web56506.mail.re3.yahoo.com>
Hi,
On Mon, Aug 23, 2010 at 3:22 AM, Shreesh Holla <hshreesh@yahoo.com> wrote:
> Hi,
>
> I'm running Ubuntu with 2.6.32.24 kernel.
> I have a simple RFCOMM program running on Windows Mobile which would connect
> into the Ubuntu machine. Of course I have the corresponding service running on
> the Ubuntu machine. The RFCOMM port numbers do match up.
>
> This was working a few days back and now with no changes(i.e. the same
> executable), it refuses to work anymore.
>
> So is there some configuration needed now?
>
> I have the dongle running in Ubuntu as : hciconfig hci0 noauth noencrypt pscan
Had you tried the latest bluez, I did some changes to pair that solve
some problem with RFCOMM and ssp. If that doesn't help maybe you
should try to produce some logs of the so we can take a look.
Regards,
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH 0/4] Fix error reporting for SDP Requests
From: Johan Hedberg @ 2010-08-24 23:15 UTC (permalink / raw)
To: angelab; +Cc: Marcel Holtmann, linux-bluetooth
In-Reply-To: <f5a72626c821919cb059a7efc6e7fb9f.squirrel@www.codeaurora.org>
Hi Angela,
On Tue, Aug 24, 2010, angelab@codeaurora.org wrote:
> Section 4.4 of the Bluetooth Corev2.1 spec, states that the error code
> shall be 0x04 when an invalid PDU size is received. BlueZ is returning
> error code 0x03 for the following:
> TP/SERVER/SS/BI-01-C [PATCH 1/4]
> TP/SERVER/SA/BI-03-C [PATCH 2/4]
> TP/SERVER/SSA/BI-02-C [PATCH 3/4]
>
> For this test case I sent a bad search attribute request where the max
> amount Of Attribute Data to return is 0, but blueZ fails to respond with
> an error code. The "max amount of attribute data" field has to be set to a
> minimum size of 0x0007 for the request to be valid.
> TP/SERVER/SA/BI-02-C [PATCH 4/4]
Ok, so it sounds like you're using your own test system for this? I
don't think Marcel was referring to what the spec says but what the BITE
tester test vectors require in practice. So far there haven't been any
issues with the BITE with respect to this. Of course it's good to not
just pass with the BITE but to also be consistent with the spec, and so
these patches seem like a good idea. However, could you resend them with
the test case reference in each commit message as well as any other
relevant info like the section in the core spec? Thanks.
Johan
^ permalink raw reply
* Re: [PATCH 0/4] Fix error reporting for SDP Requests
From: angelab @ 2010-08-24 22:55 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1282688488.6841.34.camel@localhost.localdomain>
Hi Marcel,
Section 4.4 of the Bluetooth Corev2.1 spec, states that the error code
shall be 0x04 when an invalid PDU size is received. BlueZ is returning
error code 0x03 for the following:
TP/SERVER/SS/BI-01-C [PATCH 1/4]
TP/SERVER/SA/BI-03-C [PATCH 2/4]
TP/SERVER/SSA/BI-02-C [PATCH 3/4]
For this test case I sent a bad search attribute request where the max
amount Of Attribute Data to return is 0, but blueZ fails to respond with
an error code. The "max amount of attribute data" field has to be set to a
minimum size of 0x0007 for the request to be valid.
TP/SERVER/SA/BI-02-C [PATCH 4/4]
Sincerely,
Angela Bartholomaus
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
> Hi Angela,
>
>> Fixed the following issues revealed while performing bluez
>> qualification:
>> - Incorrect error code was being reported for requests with invalid PDU
>> size;
>> - No error was being reported if a request less than 0x07 was received.
>
> I never needed any of these fixes when doing qualification. So why do
> you? Please mention the test case numbers.
>
> Regards
>
> Marcel
>
>
>
^ permalink raw reply
* [PATCH 5/5] Bluetooth: Use a stream-oriented recvmsg with SOCK_STREAM L2CAP sockets.
From: Mat Martineau @ 2010-08-24 22:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1282689346-20371-1-git-send-email-mathewm@codeaurora.org>
L2CAP ERTM sockets can be opened with the SOCK_STREAM socket type,
which is a mandatory request for ERTM mode.
However, these sockets still have SOCK_SEQPACKET read semantics when
bt_sock_recvmsg() is used to pull data from the receive queue. If the
application is only reading part of a frame, then the unread portion
of the frame is discarded. If the application requests more bytes
than are in the current frame, only the current frame's data is
returned.
This patch utilizes common code derived from RFCOMM's recvmsg()
function to make L2CAP SOCK_STREAM reads behave like RFCOMM reads (and
other SOCK_STREAM sockets in general). The application may read one
byte at a time from the input stream and not lose any data, and may
also read across L2CAP frame boundaries.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 1f1fa3c..05db244 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1960,6 +1960,9 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct ms
release_sock(sk);
+ if (sock->type == SOCK_STREAM)
+ return bt_sock_stream_recvmsg(iocb, sock, msg, len, flags);
+
return bt_sock_recvmsg(iocb, sock, msg, len, flags);
}
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH 4/5] Bluetooth: Use common SOCK_STREAM receive code in RFCOMM
From: Mat Martineau @ 2010-08-24 22:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1282689346-20371-1-git-send-email-mathewm@codeaurora.org>
To reduce code duplication, have rfcomm_sock_recvmsg() call
bt_sock_stream_recvmsg(). The common bt_sock_stream_recvmsg()
code is nearly identical, with the RFCOMM-specific functionality
for deferred setup and connection unthrottling left in
rfcomm_sock_recvmsg().
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/rfcomm/sock.c | 104 +++----------------------------------------
1 files changed, 6 insertions(+), 98 deletions(-)
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 44a6232..4396f47 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -617,121 +617,29 @@ static int rfcomm_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
return sent;
}
-static long rfcomm_sock_data_wait(struct sock *sk, long timeo)
-{
- DECLARE_WAITQUEUE(wait, current);
-
- add_wait_queue(sk_sleep(sk), &wait);
- for (;;) {
- set_current_state(TASK_INTERRUPTIBLE);
-
- if (!skb_queue_empty(&sk->sk_receive_queue) ||
- sk->sk_err ||
- (sk->sk_shutdown & RCV_SHUTDOWN) ||
- signal_pending(current) ||
- !timeo)
- break;
-
- set_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
- release_sock(sk);
- timeo = schedule_timeout(timeo);
- lock_sock(sk);
- clear_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
- }
-
- __set_current_state(TASK_RUNNING);
- remove_wait_queue(sk_sleep(sk), &wait);
- return timeo;
-}
-
static int rfcomm_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
struct msghdr *msg, size_t size, int flags)
{
struct sock *sk = sock->sk;
struct rfcomm_dlc *d = rfcomm_pi(sk)->dlc;
- int err = 0;
- size_t target, copied = 0;
- long timeo;
+ int len;
if (test_and_clear_bit(RFCOMM_DEFER_SETUP, &d->flags)) {
rfcomm_dlc_accept(d);
return 0;
}
- if (flags & MSG_OOB)
- return -EOPNOTSUPP;
-
- msg->msg_namelen = 0;
-
- BT_DBG("sk %p size %zu", sk, size);
+ len = bt_sock_stream_recvmsg(iocb, sock, msg, size, flags);
lock_sock(sk);
+ if (!(flags & MSG_PEEK) && len > 0)
+ atomic_sub(len, &sk->sk_rmem_alloc);
- target = sock_rcvlowat(sk, flags & MSG_WAITALL, size);
- timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
-
- do {
- struct sk_buff *skb;
- int chunk;
-
- skb = skb_dequeue(&sk->sk_receive_queue);
- if (!skb) {
- if (copied >= target)
- break;
-
- if ((err = sock_error(sk)) != 0)
- break;
- if (sk->sk_shutdown & RCV_SHUTDOWN)
- break;
-
- err = -EAGAIN;
- if (!timeo)
- break;
-
- timeo = rfcomm_sock_data_wait(sk, timeo);
-
- if (signal_pending(current)) {
- err = sock_intr_errno(timeo);
- goto out;
- }
- continue;
- }
-
- chunk = min_t(unsigned int, skb->len, size);
- if (memcpy_toiovec(msg->msg_iov, skb->data, chunk)) {
- skb_queue_head(&sk->sk_receive_queue, skb);
- if (!copied)
- copied = -EFAULT;
- break;
- }
- copied += chunk;
- size -= chunk;
-
- sock_recv_ts_and_drops(msg, sk, skb);
-
- if (!(flags & MSG_PEEK)) {
- atomic_sub(chunk, &sk->sk_rmem_alloc);
-
- skb_pull(skb, chunk);
- if (skb->len) {
- skb_queue_head(&sk->sk_receive_queue, skb);
- break;
- }
- kfree_skb(skb);
-
- } else {
- /* put message back and return */
- skb_queue_head(&sk->sk_receive_queue, skb);
- break;
- }
- } while (size);
-
-out:
if (atomic_read(&sk->sk_rmem_alloc) <= (sk->sk_rcvbuf >> 2))
rfcomm_dlc_unthrottle(rfcomm_pi(sk)->dlc);
-
release_sock(sk);
- return copied ? : err;
+
+ return len;
}
static int rfcomm_sock_setsockopt_old(struct socket *sock, int optname, char __user *optval, unsigned int optlen)
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH 3/5] Bluetooth: Add common code for stream-oriented recvmsg()
From: Mat Martineau @ 2010-08-24 22:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1282689346-20371-1-git-send-email-mathewm@codeaurora.org>
This commit adds a bt_sock_stream_recvmsg() function for use by any
Bluetooth code that uses SOCK_STREAM sockets. This code is copied
from rfcomm_sock_recvmsg() with minimal modifications to remove
RFCOMM-specific functionality and improve readability.
L2CAP (with the SOCK_STREAM socket type) and RFCOMM have common needs
when it comes to reading data. Proper stream read semantics require
that applications can read from a stream one byte at a time and not
lose any data. The RFCOMM code already operated on and pulled data
from the underlying L2CAP socket, so very few changes were required to
make the code more generic for use with non-RFCOMM data over L2CAP.
Applications that need more awareness of L2CAP frame boundaries are
still free to use SOCK_SEQPACKET sockets, and may verify that they
connection did not fall back to basic mode by calling getsockopt().
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
include/net/bluetooth/bluetooth.h | 2 +
net/bluetooth/af_bluetooth.c | 109 +++++++++++++++++++++++++++++++++++++
2 files changed, 111 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 27a902d..08b6c2a 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -126,6 +126,8 @@ int bt_sock_unregister(int proto);
void bt_sock_link(struct bt_sock_list *l, struct sock *s);
void bt_sock_unlink(struct bt_sock_list *l, struct sock *s);
int bt_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg, size_t len, int flags);
+int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
+ struct msghdr *msg, size_t len, int flags);
uint bt_sock_poll(struct file * file, struct socket *sock, poll_table *wait);
int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 421c45b..77a26fe 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -265,6 +265,115 @@ int bt_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
}
EXPORT_SYMBOL(bt_sock_recvmsg);
+static long bt_sock_data_wait(struct sock *sk, long timeo)
+{
+ DECLARE_WAITQUEUE(wait, current);
+
+ add_wait_queue(sk_sleep(sk), &wait);
+ for (;;) {
+ set_current_state(TASK_INTERRUPTIBLE);
+
+ if (!skb_queue_empty(&sk->sk_receive_queue))
+ break;
+
+ if (sk->sk_err || (sk->sk_shutdown & RCV_SHUTDOWN))
+ break;
+
+ if (signal_pending(current) || !timeo)
+ break;
+
+ set_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
+ release_sock(sk);
+ timeo = schedule_timeout(timeo);
+ lock_sock(sk);
+ clear_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
+ }
+
+ __set_current_state(TASK_RUNNING);
+ remove_wait_queue(sk_sleep(sk), &wait);
+ return timeo;
+}
+
+int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
+ struct msghdr *msg, size_t size, int flags)
+{
+ struct sock *sk = sock->sk;
+ int err = 0;
+ size_t target, copied = 0;
+ long timeo;
+
+ if (flags & MSG_OOB)
+ return -EOPNOTSUPP;
+
+ msg->msg_namelen = 0;
+
+ BT_DBG("sk %p size %zu", sk, size);
+
+ lock_sock(sk);
+
+ target = sock_rcvlowat(sk, flags & MSG_WAITALL, size);
+ timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
+
+ do {
+ struct sk_buff *skb;
+ int chunk;
+
+ skb = skb_dequeue(&sk->sk_receive_queue);
+ if (!skb) {
+ if (copied >= target)
+ break;
+
+ if ((err = sock_error(sk)) != 0)
+ break;
+ if (sk->sk_shutdown & RCV_SHUTDOWN)
+ break;
+
+ err = -EAGAIN;
+ if (!timeo)
+ break;
+
+ timeo = bt_sock_data_wait(sk, timeo);
+
+ if (signal_pending(current)) {
+ err = sock_intr_errno(timeo);
+ goto out;
+ }
+ continue;
+ }
+
+ chunk = min_t(unsigned int, skb->len, size);
+ if (memcpy_toiovec(msg->msg_iov, skb->data, chunk)) {
+ skb_queue_head(&sk->sk_receive_queue, skb);
+ if (!copied)
+ copied = -EFAULT;
+ break;
+ }
+ copied += chunk;
+ size -= chunk;
+
+ sock_recv_ts_and_drops(msg, sk, skb);
+
+ if (!(flags & MSG_PEEK)) {
+ skb_pull(skb, chunk);
+ if (skb->len) {
+ skb_queue_head(&sk->sk_receive_queue, skb);
+ break;
+ }
+ kfree_skb(skb);
+
+ } else {
+ /* put message back and return */
+ skb_queue_head(&sk->sk_receive_queue, skb);
+ break;
+ }
+ } while (size);
+
+out:
+ release_sock(sk);
+ return copied ? : err;
+}
+EXPORT_SYMBOL(bt_sock_stream_recvmsg);
+
static inline unsigned int bt_accept_poll(struct sock *parent)
{
struct list_head *p, *n;
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH 2/5] Bluetooth: Validate PSM values in calls to connect() and bind()
From: Mat Martineau @ 2010-08-24 22:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1282689346-20371-1-git-send-email-mathewm@codeaurora.org>
Valid L2CAP PSMs are odd numbers, and the least significant bit of the
most significant byte must be 0.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap.c | 24 ++++++++++++++++++++----
1 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index c784703..1f1fa3c 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1008,10 +1008,20 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
goto done;
}
- if (la.l2_psm && __le16_to_cpu(la.l2_psm) < 0x1001 &&
- !capable(CAP_NET_BIND_SERVICE)) {
- err = -EACCES;
- goto done;
+ if (la.l2_psm) {
+ __u16 psm = __le16_to_cpu(la.l2_psm);
+
+ /* PSM must be odd and lsb of upper byte must be 0 */
+ if ((psm & 0x0101) != 0x0001) {
+ err = -EINVAL;
+ goto done;
+ }
+
+ /* Restrict usage of well-known PSMs */
+ if (psm < 0x1001 && !capable(CAP_NET_BIND_SERVICE)) {
+ err = -EACCES;
+ goto done;
+ }
}
write_lock_bh(&l2cap_sk_list.lock);
@@ -1190,6 +1200,12 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al
goto done;
}
+ /* PSM must be odd and lsb of upper byte must be 0 */
+ if ((__le16_to_cpu(la.l2_psm) & 0x0101) != 0x0001) {
+ err = -EINVAL;
+ goto done;
+ }
+
/* Set destination address and psm */
bacpy(&bt_sk(sk)->dst, &la.l2_bdaddr);
l2cap_pi(sk)->psm = la.l2_psm;
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH 1/5] Bluetooth: Only enable for L2CAP FCS for ERTM or streaming
From: Mat Martineau @ 2010-08-24 22:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1282689346-20371-1-git-send-email-mathewm@codeaurora.org>
This fixes a bug which caused the FCS setting to show L2CAP_FCS_CRC16
with L2CAP modes other than ERTM or streaming. At present, this only
affects the FCS value shown with getsockopt() for basic mode.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap.c | 19 +++++++++++++------
1 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index fadf26b..c784703 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3071,6 +3071,17 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd
return 0;
}
+static inline void set_default_fcs(struct l2cap_pinfo *pi)
+{
+ /* FCS is enabled only in ERTM or streaming mode, if one or both
+ * sides request it.
+ */
+ if (pi->mode != L2CAP_MODE_ERTM && pi->mode != L2CAP_MODE_STREAMING)
+ pi->fcs = L2CAP_FCS_NONE;
+ else if (!(pi->conf_state & L2CAP_CONF_NO_FCS_RECV))
+ pi->fcs = L2CAP_FCS_CRC16;
+}
+
static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, u16 cmd_len, u8 *data)
{
struct l2cap_conf_req *req = (struct l2cap_conf_req *) data;
@@ -3135,9 +3146,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
goto unlock;
if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
- if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
- l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
- l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
+ set_default_fcs(l2cap_pi(sk));
sk->sk_state = BT_CONNECTED;
@@ -3225,9 +3234,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
l2cap_pi(sk)->conf_state |= L2CAP_CONF_INPUT_DONE;
if (l2cap_pi(sk)->conf_state & L2CAP_CONF_OUTPUT_DONE) {
- if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
- l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
- l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
+ set_default_fcs(l2cap_pi(sk));
sk->sk_state = BT_CONNECTED;
l2cap_pi(sk)->next_tx_seq = 0;
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH v5 0/5] L2CAP updates for FCS, valid PSMs, and stream recv
From: Mat Martineau @ 2010-08-24 22:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm
These patches incorporate requested fixes from the 'v4' submission.
[PATCH 1/5] Bluetooth: Only enable for L2CAP FCS for ERTM or streaming
This got an "applied" response on the mailing list, but it looks like
the commit got dropped during a rebase.
[PATCH 2/5] Bluetooth: Validate PSM values in calls to connect() and bind()
Fixed comments.
[PATCH 3/5] Bluetooth: Add common code for stream-oriented recvmsg()
[PATCH 4/5] Bluetooth: Use common SOCK_STREAM receive code in RFCOMM
[PATCH 5/5] Bluetooth: Use a stream-oriented recvmsg with SOCK_STREAM L2CAP sockets.
No changes were requested in these patches.
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ 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