* [PATCH 3/5] cltest: Fix memory leak
From: Andre Guedes @ 2014-02-07 22:28 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1391812125-19594-1-git-send-email-andre.guedes@openbossa.org>
---
tools/cltest.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/cltest.c b/tools/cltest.c
index 16b7553..4ddb98a 100644
--- a/tools/cltest.c
+++ b/tools/cltest.c
@@ -205,8 +205,8 @@ static bool find_controllers(void)
dl = malloc(HCI_MAX_DEV * sizeof(struct hci_dev_req) + sizeof(uint16_t));
if (!dl) {
perror("Failed allocate HCI device request memory");
- result = false;
- goto done;
+ close(fd);
+ return false;
}
dl->dev_num = HCI_MAX_DEV;
@@ -243,6 +243,7 @@ static bool find_controllers(void)
}
done:
+ free(dl);
close(fd);
return result;
}
--
1.8.5.3
^ permalink raw reply related
* [PATCH 4/5] amptest: Fix memory leak
From: Andre Guedes @ 2014-02-07 22:28 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1391812125-19594-1-git-send-email-andre.guedes@openbossa.org>
---
tools/amptest.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/tools/amptest.c b/tools/amptest.c
index 16f15bc..6192f7e 100644
--- a/tools/amptest.c
+++ b/tools/amptest.c
@@ -496,6 +496,7 @@ static bool find_amp_controller(void)
struct hci_dev_list_req *dl;
struct hci_dev_req *dr;
int fd, i;
+ bool result;
fd = socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI);
if (fd < 0) {
@@ -515,8 +516,8 @@ static bool find_amp_controller(void)
if (ioctl(fd, HCIGETDEVLIST, (void *) dl) < 0) {
perror("Failed to get HCI device list");
- close(fd);
- return false;
+ result = false;
+ goto done;
}
for (i = 0; i< dl->dev_num; i++) {
@@ -541,9 +542,12 @@ static bool find_amp_controller(void)
}
}
- close(fd);
+ result = true;
- return true;
+done:
+ free(dl);
+ close(fd);
+ return result;
}
int main(int argc ,char *argv[])
--
1.8.5.3
^ permalink raw reply related
* [PATCH 5/5] rctest: Fix memory leak
From: Andre Guedes @ 2014-02-07 22:28 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1391812125-19594-1-git-send-email-andre.guedes@openbossa.org>
---
tools/rctest.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/rctest.c b/tools/rctest.c
index 77fa03c..9281392 100644
--- a/tools/rctest.c
+++ b/tools/rctest.c
@@ -466,8 +466,11 @@ static void save_mode(int sk)
while ((len = read(sk, b, data_size)) > 0) {
ret = write(save_fd, b, len);
if (ret < 0)
- return;
+ goto done;
}
+
+done:
+ free(b);
}
static void recv_mode(int sk)
--
1.8.5.3
^ permalink raw reply related
* RE: possible bug in blueZ 5.8 gatt tool or library
From: Caleb Reinhold @ 2014-02-07 23:34 UTC (permalink / raw)
To: 'Anderson Lizardo'; +Cc: 'BlueZ development'
In-Reply-To: <CAJdJm_OeMUO5Qj3Amg7ADtkJRsRBpoixD2MEBBWHUA3V1M1OqA@mail.gmail.com>
Hi Anderson,
>If I remember correctly, the issue is in the kernel: if connect() is called
when security level is medium,
>the socket only gets POLLOUT once SMP pairing finishes, and any ATT PDU
received during that time is lost.
This is useful and interesting to know.
>Note that it's almost certain that your device is sending the indication
without requiring encryption.
>Otherwise, it would have sent a Security Request (which triggers a Pairing
Request from the Linux side)
>and wait for the encryption to be enabled before sending the indication. If
that was the case,
>the kernel would deliver the ATT PDU to gatttool after encryption is
enabled and it would work as expected.
It seems our device is indeed sending the indication without requiring
encryption. The suggestion of updating the central device to start with low
security and then change to medium security is producing the desired
behavior. Thank you for your time and assistance.
Regards,
Caleb Reinhold
^ permalink raw reply
* Some patches applied on Fedora that maybe should be considered for being applied upstream
From: Pacho Ramos @ 2014-02-09 8:53 UTC (permalink / raw)
To: BlueZ development
Hello
I was looking at bluez package and found some patches that maybe could
be upstreamed. Also, I would like to know the reasons for not accepting
them to ensure they are safe to be applied downstream by us too :)
http://pkgs.fedoraproject.org/cgit/bluez.git/tree/0001-Allow-using-obexd-without-systemd-in-the-user-sessio.patch -> Does this cause any issues with systemd --user setups?
http://pkgs.fedoraproject.org/cgit/bluez.git/tree/0001-obex-Use-GLib-helper-function-to-manipulate-paths.patch
http://pkgs.fedoraproject.org/cgit/bluez.git/tree/0002-autopair-Don-t-handle-the-iCade.patch
http://pkgs.fedoraproject.org/cgit/bluez.git/tree/0004-agent-Assert-possible-infinite-loop.patch
-> Any reason for not applying it upstream too?
http://pkgs.fedoraproject.org/cgit/bluez.git/tree/0001-work-around-Logitech-diNovo-Edge-keyboard-firmware-i.patch
-> Taking care this looks to be a really old issue, maybe using the
workaround would be the only option for now :/
Thanks a lot for your thoughts :)
^ permalink raw reply
* Bluez 5, A2DP headphones
From: Pavel Volkov @ 2014-02-09 20:37 UTC (permalink / raw)
To: linux-bluetooth
I'm having trouble connecting to my headphones after upgrading from Bluez 4 to
Bluez 5 with a few more people.
Link to Gentoo Bugzilla: https://bugs.gentoo.org/show_bug.cgi?id=499908
Summary:
I'm not using PulseAudio and this is the error from system journal:
2月 09 15:01:54 melforce bluetoothd[12825]: a2dp-sink profile connect failed for
00:02:3C:31:96:12: Protocol not available
It appears when I try to connect to it from bluedevil (KDE's Bluetooth stack)
or when mplayer2 tries to open the ALSA device.
Do you have any clues?
^ permalink raw reply
* [PATCH 00/24] rfcomm fixes
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
Marcel,
This patch series addresses a number of previously unknown issues
with the RFCOMM tty device implementation, in addition to
addressing the locking regression recently reported [1].
As Gianluca suggested and I agree, this series first reverts
3 of the 4 patches of 3.14-rc1 for bluetooth/rfcomm/tty.c.
The reasoning is detailed in the changelog for
Revert "Bluetooth: Always wait for a connection on RFCOMM open()"
but the short answer is that it re-implements a long-standing
bug by blocking on a non-blocking open.
This patch series corrects the reported regressions from 3.13
(to the extent that correction is required). Specifically,
the ModemManager regression reported by Gianluca Anzolin [2]
and the rfcomm bind with wvdial reported by Andrey Vihrov [3].
tty: Fix ref counting for port krefs
Bluetooth: Fix racy acquire of rfcomm_dev reference
Bluetooth: Exclude released devices from RFCOMMGETDEVLIST ioctl
Bluetooth: Release rfcomm_dev only once
Bluetooth: Fix unreleased rfcomm_dev reference
These first 5 patches after the reverts
fix 4 different rfcomm_dev ref count mishandling bugs.
Bluetooth: Fix RFCOMM tty teardown race and
Bluetooth: Serialize RFCOMMCREATEDEV and RFCOMMRELEASEDEV ioctls
Fix races which occur due to the design of the rfcomm ioctls
(note that buses don't have these kinds of races).
Bluetooth: Verify dlci not in use before rfcomm_dev create
Bluetooth: Simplify RFCOMM session state eval
Bluetooth: Refactor deferred setup test in rfcomm_dlc_close()
Bluetooth: Refactor dlc disconnect logic in rfcomm_dlc_close()
Bluetooth: Directly close dlc for not yet started RFCOMM session
These 5 patches fix issues with reusing the dlci after
closing the tty (found by unit test).
Bluetooth: Fix unsafe RFCOMM device parenting
Bluetooth: Fix RFCOMM parent device for reused dlc
These 2 patches fix the ModemManager regression.
Bluetooth: Refactor rfcomm_dev_add()
Bluetooth: Cleanup RFCOMM device registration error handling
These 2 patches fix an unreleased module reference while
error handling.
Bluetooth: Rename __rfcomm_dev_get() to __rfcomm_dev_lookup()
This is a trivial naming patch with no functional impact.
Bluetooth: Force -EIO from tty read/write if .activate() fails
The tty core provides an existing mechanism for failing
reads/writes if device activation fails (like an error
allocating the dlc).
Bluetooth: Don't fail RFCOMM tty writes
This patch implements buffered writes even if the device
is not connected.
While unit testing this, I discovered a serious defect in
the way available space is computed that under-utilizes
rfcomm i/o and may even halt further tx on that link, which
is fixed by:
Bluetooth: Refactor write_room() calculation
Bluetooth: Fix write_room() calculation
Note that this series does not fix the naively inefficient
method of packetizing tty output; packetizing should be
done on the krfcommd thread to take advantage of aggregating
multiple tty writes into 1 or more packets. Look at any
line-by-line console output to realize how under-utilized
the rfcomm tty packeting is.
[1] http://www.spinics.net/lists/linux-wireless/msg117818.html
[2] http://www.spinics.net/lists/linux-bluetooth/msg42075.html
[3] http://www.spinics.net/lists/linux-bluetooth/msg42057.html
Regards,
Peter Hurley (24):
Revert "Bluetooth: Remove rfcomm_carrier_raised()"
Revert "Bluetooth: Always wait for a connection on RFCOMM open()"
Revert "Bluetooth: Move rfcomm_get_device() before
rfcomm_dev_activate()"
tty: Fix ref counting for port krefs
Bluetooth: Fix racy acquire of rfcomm_dev reference
Bluetooth: Exclude released devices from RFCOMMGETDEVLIST ioctl
Bluetooth: Release rfcomm_dev only once
Bluetooth: Fix unreleased rfcomm_dev reference
Bluetooth: Fix RFCOMM tty teardown race
Bluetooth: Verify dlci not in use before rfcomm_dev create
Bluetooth: Simplify RFCOMM session state eval
Bluetooth: Refactor deferred setup test in rfcomm_dlc_close()
Bluetooth: Refactor dlc disconnect logic in rfcomm_dlc_close()
Bluetooth: Directly close dlc for not yet started RFCOMM session
Bluetooth: Fix unsafe RFCOMM device parenting
Bluetooth: Fix RFCOMM parent device for reused dlc
Bluetooth: Rename __rfcomm_dev_get() to __rfcomm_dev_lookup()
Bluetooth: Serialize RFCOMMCREATEDEV and RFCOMMRELEASEDEV ioctls
Bluetooth: Refactor rfcomm_dev_add()
Bluetooth: Cleanup RFCOMM device registration error handling
Bluetooth: Force -EIO from tty read/write if .activate() fails
Bluetooth: Don't fail RFCOMM tty writes
Bluetooth: Refactor write_room() calculation
Bluetooth: Fix write_room() calculation
include/linux/tty.h | 6 +-
include/net/bluetooth/rfcomm.h | 9 +-
net/bluetooth/rfcomm/core.c | 88 ++++++++++----
net/bluetooth/rfcomm/tty.c | 262 ++++++++++++++++++++++-------------------
4 files changed, 223 insertions(+), 142 deletions(-)
--
1.8.1.2
^ permalink raw reply
* [PATCH 01/24] Revert "Bluetooth: Remove rfcomm_carrier_raised()"
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
This reverts commit f86772af6a0f643d3e13eb3f4f9213ae0c333ee4.
This is the first of a 3-patch revert, together with
Revert "Bluetooth: Always wait for a connection on RFCOMM open()" and
Revert "Bluetooth: Move rfcomm_get_device() before rfcomm_dev_activate()".
Commit 4a2fb3ecc7467c775b154813861f25a0ddc11aa0,
"Bluetooth: Always wait for a connection on RFCOMM open()" open-codes
blocking on tty open(), rather than using the default behavior
implemented by the tty port.
The reasons for reverting that patch are detailed in that changelog;
this patch restores required functionality for that revert.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
net/bluetooth/rfcomm/tty.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index f9c0980a..aeabade 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -160,6 +160,14 @@ static int rfcomm_dev_activate(struct tty_port *port, struct tty_struct *tty)
return err;
}
+/* we block the open until the dlc->state becomes BT_CONNECTED */
+static int rfcomm_dev_carrier_raised(struct tty_port *port)
+{
+ struct rfcomm_dev *dev = container_of(port, struct rfcomm_dev, port);
+
+ return (dev->dlc->state == BT_CONNECTED);
+}
+
/* device-specific cleanup: close the dlc */
static void rfcomm_dev_shutdown(struct tty_port *port)
{
--
1.8.1.2
^ permalink raw reply related
* [PATCH 02/24] Revert "Bluetooth: Always wait for a connection on RFCOMM open()"
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
This reverts commit 4a2fb3ecc7467c775b154813861f25a0ddc11aa0.
This is the second of a 3-patch revert, together with
Revert "Bluetooth: Remove rfcomm_carrier_raised()" and
Revert "Bluetooth: Move rfcomm_get_device() before rfcomm_dev_activate()".
Before commit cad348a17e170451ea8688b532a6ca3e98c63b60,
Bluetooth: Implement .activate, .shutdown and .carrier_raised methods,
tty_port_block_til_ready() was open-coded in rfcomm_tty_install() as
part of the RFCOMM tty open().
Unfortunately, it did not implement non-blocking open nor CLOCAL open,
but rather always blocked for carrier. This is not the expected or
typical behavior for ttys, and prevents several common terminal
programming idioms from working (eg., opening in non-blocking
mode to initialize desired termios settings then re-opening for
connection).
Commit cad348a17e170451ea8688b532a6ca3e98c63b60,
Bluetooth: Implement .activate, .shutdown and .carrier_raised methods,
added the necessary tty_port methods to use the default tty_port_open().
However, this triggered two important user-space regressions.
The first regression involves the complicated mechanism for reparenting
the rfcomm tty device to the ACL link device which represents an
open link to a specific bluetooth host. This regression causes ModemManager
to conclude the rfcomm tty device does not front a modem so it makes
no attempt to initialize an attached modem. This regression is
caused by the lack of a device_move() if the dlc is already open (and
not specifically related to the open-coded block_til_ready()).
A more appropriate solution is submitted in
"Bluetooth: Fix unsafe RFCOMM device parenting" and
"Bluetooth: Fix RFCOMM parent device for reused dlc"
The second regression involves "rfcomm bind" and wvdial (a ppp dialer).
rfcomm bind creates a device node for a /dev/rfcomm<n>. wvdial opens
that device in non-blocking mode (because it expects the connection
to have already been established). In addition, subsequent writes
to the rfcomm tty device fail (because the link is not yet connected;
rfcomm connection begins with the actual tty open()).
However, restoring the original behavior (in the patch which
this reverts) was undesirable.
Firstly, the original reporter notes that a trivial userspace
"workaround" already exists: rfcomm connect, which creates the
device node and establishes the expected connection.
Secondly, the failed writes occur because the rfcomm tty driver
does not buffer writes to an unconnected device; this contrasts with
the dozen of other tty drivers (in fact, all of them) that do just
that. The submitted patch "Bluetooth: Don't fail RFCOMM tty writes"
corrects this.
Thirdly, it was a long-standing bug to block on non-blocking open,
which is re-fixed by revert.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
net/bluetooth/rfcomm/tty.c | 46 ++++++++--------------------------------------
1 file changed, 8 insertions(+), 38 deletions(-)
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index aeabade..32ef9f9 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -58,7 +58,6 @@ struct rfcomm_dev {
uint modem_status;
struct rfcomm_dlc *dlc;
- wait_queue_head_t conn_wait;
struct device *tty_dev;
@@ -124,40 +123,8 @@ static struct device *rfcomm_get_device(struct rfcomm_dev *dev)
static int rfcomm_dev_activate(struct tty_port *port, struct tty_struct *tty)
{
struct rfcomm_dev *dev = container_of(port, struct rfcomm_dev, port);
- DEFINE_WAIT(wait);
- int err;
-
- err = rfcomm_dlc_open(dev->dlc, &dev->src, &dev->dst, dev->channel);
- if (err)
- return err;
-
- while (1) {
- prepare_to_wait(&dev->conn_wait, &wait, TASK_INTERRUPTIBLE);
- if (dev->dlc->state == BT_CLOSED) {
- err = -dev->err;
- break;
- }
-
- if (dev->dlc->state == BT_CONNECTED)
- break;
-
- if (signal_pending(current)) {
- err = -ERESTARTSYS;
- break;
- }
-
- tty_unlock(tty);
- schedule();
- tty_lock(tty);
- }
- finish_wait(&dev->conn_wait, &wait);
-
- if (!err)
- device_move(dev->tty_dev, rfcomm_get_device(dev),
- DPM_ORDER_DEV_AFTER_PARENT);
-
- return err;
+ return rfcomm_dlc_open(dev->dlc, &dev->src, &dev->dst, dev->channel);
}
/* we block the open until the dlc->state becomes BT_CONNECTED */
@@ -184,6 +151,7 @@ static const struct tty_port_operations rfcomm_port_ops = {
.destruct = rfcomm_dev_destruct,
.activate = rfcomm_dev_activate,
.shutdown = rfcomm_dev_shutdown,
+ .carrier_raised = rfcomm_dev_carrier_raised,
};
static struct rfcomm_dev *__rfcomm_dev_get(int id)
@@ -290,7 +258,6 @@ static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
tty_port_init(&dev->port);
dev->port.ops = &rfcomm_port_ops;
- init_waitqueue_head(&dev->conn_wait);
skb_queue_head_init(&dev->pending);
@@ -609,9 +576,12 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
BT_DBG("dlc %p dev %p err %d", dlc, dev, err);
dev->err = err;
- wake_up_interruptible(&dev->conn_wait);
+ if (dlc->state == BT_CONNECTED) {
+ device_move(dev->tty_dev, rfcomm_get_device(dev),
+ DPM_ORDER_DEV_AFTER_PARENT);
- if (dlc->state == BT_CLOSED)
+ wake_up_interruptible(&dev->port.open_wait);
+ } else if (dlc->state == BT_CLOSED)
tty_port_tty_hangup(&dev->port, false);
}
@@ -1133,7 +1103,7 @@ int __init rfcomm_init_ttys(void)
rfcomm_tty_driver->subtype = SERIAL_TYPE_NORMAL;
rfcomm_tty_driver->flags = TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
rfcomm_tty_driver->init_termios = tty_std_termios;
- rfcomm_tty_driver->init_termios.c_cflag = B9600 | CS8 | CREAD | HUPCL | CLOCAL;
+ rfcomm_tty_driver->init_termios.c_cflag = B9600 | CS8 | CREAD | HUPCL;
rfcomm_tty_driver->init_termios.c_lflag &= ~ICANON;
tty_set_operations(rfcomm_tty_driver, &rfcomm_ops);
--
1.8.1.2
^ permalink raw reply related
* [PATCH 03/24] Revert "Bluetooth: Move rfcomm_get_device() before rfcomm_dev_activate()"
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
This reverts commit e228b63390536f5b737056059a9a04ea016b1abf.
This is the third of a 3-patch revert, together with
Revert "Bluetooth: Remove rfcomm_carrier_raised()" and
Revert "Bluetooth: Always wait for a connection on RFCOMM open()".
Commit 4a2fb3ecc7467c775b154813861f25a0ddc11aa0,
"Bluetooth: Always wait for a connection on RFCOMM open()" open-codes
blocking on tty open(), rather than using the default behavior
implemented by the tty port.
The reasons for reverting that patch are detailed in that changelog;
this patch restores required functionality for that revert.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
net/bluetooth/rfcomm/tty.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 32ef9f9..a535ef1 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -103,22 +103,6 @@ static void rfcomm_dev_destruct(struct tty_port *port)
module_put(THIS_MODULE);
}
-static struct device *rfcomm_get_device(struct rfcomm_dev *dev)
-{
- struct hci_dev *hdev;
- struct hci_conn *conn;
-
- hdev = hci_get_route(&dev->dst, &dev->src);
- if (!hdev)
- return NULL;
-
- conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &dev->dst);
-
- hci_dev_put(hdev);
-
- return conn ? &conn->dev : NULL;
-}
-
/* device-specific initialization: open the dlc */
static int rfcomm_dev_activate(struct tty_port *port, struct tty_struct *tty)
{
@@ -185,6 +169,22 @@ static struct rfcomm_dev *rfcomm_dev_get(int id)
return dev;
}
+static struct device *rfcomm_get_device(struct rfcomm_dev *dev)
+{
+ struct hci_dev *hdev;
+ struct hci_conn *conn;
+
+ hdev = hci_get_route(&dev->dst, &dev->src);
+ if (!hdev)
+ return NULL;
+
+ conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &dev->dst);
+
+ hci_dev_put(hdev);
+
+ return conn ? &conn->dev : NULL;
+}
+
static ssize_t show_address(struct device *tty_dev, struct device_attribute *attr, char *buf)
{
struct rfcomm_dev *dev = dev_get_drvdata(tty_dev);
--
1.8.1.2
^ permalink raw reply related
* [PATCH 04/24] tty: Fix ref counting for port krefs
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley, Jiri Slaby,
Greg Kroah-Hartman
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
The tty core supports two models for handling tty_port lifetimes;
the tty_port can use the kref supplied by tty_port (which will
automatically destruct the tty_port when the ref count drops to
zero) or it can destruct the tty_port manually.
For tty drivers that choose to use the port kref to manage the
tty_port lifetime, it is not possible to safely acquire a port
reference conditionally. If the last reference is released after
evaluating the condition but before acquiring the reference, a
bogus reference will be held while the tty_port destruction
commences.
Rather, only acquire a port reference if the ref count is non-zero
and allow the caller to distinguish if a reference has successfully
been acquired.
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
include/linux/tty.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 90b4fdc..4781d7b 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -518,9 +518,9 @@ extern void tty_port_put(struct tty_port *port);
static inline struct tty_port *tty_port_get(struct tty_port *port)
{
- if (port)
- kref_get(&port->kref);
- return port;
+ if (port && kref_get_unless_zero(&port->kref))
+ return port;
+ return NULL;
}
/* If the cts flow control is enabled, return true. */
--
1.8.1.2
^ permalink raw reply related
* [PATCH 05/24] Bluetooth: Fix racy acquire of rfcomm_dev reference
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley, Jiri Slaby,
Greg Kroah-Hartman
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
rfcomm_dev_get() can return a rfcomm_dev reference for a
device for which destruction may be commencing. This can happen
on tty destruction, which calls rfcomm_tty_cleanup(), the last
port reference may have been released but RFCOMM_TTY_RELEASED
was not set. The following race is also possible:
CPU 0 | CPU 1
| rfcomm_release_dev
rfcomm_dev_get | .
spin_lock | .
dev = __rfcomm_dev_get | .
if dev | .
if test_bit(TTY_RELEASED) | .
| !test_and_set_bit(TTY_RELEASED)
| tty_port_put <<<< last reference
else |
tty_port_get |
The reference acquire is bogus because destruction will commence
with the release of the last reference.
Ignore the external state change of TTY_RELEASED and instead rely
on the reference acquire itself to determine if the reference is
valid.
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
net/bluetooth/rfcomm/tty.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index a535ef1..7cf193f 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -157,12 +157,8 @@ static struct rfcomm_dev *rfcomm_dev_get(int id)
dev = __rfcomm_dev_get(id);
- if (dev) {
- if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
- dev = NULL;
- else
- tty_port_get(&dev->port);
- }
+ if (dev && !tty_port_get(&dev->port))
+ dev = NULL;
spin_unlock(&rfcomm_dev_lock);
--
1.8.1.2
^ permalink raw reply related
* [PATCH 06/24] Bluetooth: Exclude released devices from RFCOMMGETDEVLIST ioctl
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
When enumerating RFCOMM devices in the rfcomm_dev_list, holding
the rfcomm_dev_lock only guarantees the existence of the enumerated
rfcomm_dev in memory, and not safe access to its state. Testing
the device state (such as RFCOMM_TTY_RELEASED) does not guarantee
the device will remain in that state for the subsequent access
to the rfcomm_dev's fields, nor guarantee that teardown has not
commenced.
Obtain an rfcomm_dev reference for the duration of rfcomm_dev
access.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
net/bluetooth/rfcomm/tty.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 7cf193f..b385d99 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -468,7 +468,7 @@ static int rfcomm_get_dev_list(void __user *arg)
spin_lock(&rfcomm_dev_lock);
list_for_each_entry(dev, &rfcomm_dev_list, list) {
- if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
+ if (!tty_port_get(&dev->port))
continue;
(di + n)->id = dev->id;
(di + n)->flags = dev->flags;
@@ -476,6 +476,7 @@ static int rfcomm_get_dev_list(void __user *arg)
(di + n)->channel = dev->channel;
bacpy(&(di + n)->src, &dev->src);
bacpy(&(di + n)->dst, &dev->dst);
+ tty_port_put(&dev->port);
if (++n >= dev_num)
break;
}
--
1.8.1.2
^ permalink raw reply related
* [PATCH 07/24] Bluetooth: Release rfcomm_dev only once
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
No logic prevents an rfcomm_dev from being released multiple
times. For example, if the rfcomm_dev ref count is large due
to pending tx, then multiple RFCOMMRELEASEDEV ioctls may
mistakenly release the rfcomm_dev too many times. Note that
concurrent ioctls are not required to create this condition.
Introduce RFCOMM_DEV_RELEASED status bit which guarantees the
rfcomm_dev can only be released once.
NB: Since the flags are exported to userspace, introduce the status
field to track state for which userspace should not be aware.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
include/net/bluetooth/rfcomm.h | 6 +++++-
net/bluetooth/rfcomm/tty.c | 11 +++++++++--
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 486213a..29d9727 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -323,11 +323,15 @@ int rfcomm_connect_ind(struct rfcomm_session *s, u8 channel,
#define RFCOMMGETDEVINFO _IOR('R', 211, int)
#define RFCOMMSTEALDLC _IOW('R', 220, int)
+/* rfcomm_dev.flags bit definitions */
#define RFCOMM_REUSE_DLC 0
#define RFCOMM_RELEASE_ONHUP 1
#define RFCOMM_HANGUP_NOW 2
#define RFCOMM_TTY_ATTACHED 3
-#define RFCOMM_TTY_RELEASED 4
+#define RFCOMM_DEFUNCT_BIT4 4 /* don't reuse this bit - userspace visible */
+
+/* rfcomm_dev.status bit definitions */
+#define RFCOMM_DEV_RELEASED 0
struct rfcomm_dev_req {
s16 dev_id;
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index b385d99..d9d4bc8 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -51,6 +51,8 @@ struct rfcomm_dev {
unsigned long flags;
int err;
+ unsigned long status; /* don't export to userspace */
+
bdaddr_t src;
bdaddr_t dst;
u8 channel;
@@ -423,6 +425,12 @@ static int rfcomm_release_dev(void __user *arg)
return -EPERM;
}
+ /* only release once */
+ if (test_and_set_bit(RFCOMM_DEV_RELEASED, &dev->status)) {
+ tty_port_put(&dev->port);
+ return -EALREADY;
+ }
+
if (req.flags & (1 << RFCOMM_HANGUP_NOW))
rfcomm_dlc_close(dev->dlc, 0);
@@ -433,8 +441,7 @@ static int rfcomm_release_dev(void __user *arg)
tty_kref_put(tty);
}
- if (!test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags) &&
- !test_and_set_bit(RFCOMM_TTY_RELEASED, &dev->flags))
+ if (!test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags))
tty_port_put(&dev->port);
tty_port_put(&dev->port);
--
1.8.1.2
^ permalink raw reply related
* [PATCH 08/24] Bluetooth: Fix unreleased rfcomm_dev reference
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
When RFCOMM_RELEASE_ONHUP is set, the rfcomm tty driver 'takes over'
the initial rfcomm_dev reference created by the RFCOMMCREATEDEV ioctl.
The assumption is that the rfcomm tty driver will release the
rfcomm_dev reference when the tty is freed (in rfcomm_tty_cleanup()).
However, if the tty is never opened, the 'take over' never occurs,
so when RFCOMMRELEASEDEV ioctl is called, the reference is not
released.
Track the state of the reference 'take over' so that the release
is guaranteed by either the RFCOMMRELEASEDEV ioctl or the rfcomm tty
driver.
Note that the synchronous hangup in rfcomm_release_dev() ensures
that rfcomm_tty_install() cannot race with the RFCOMMRELEASEDEV ioctl.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
include/net/bluetooth/rfcomm.h | 1 +
net/bluetooth/rfcomm/tty.c | 6 ++++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 29d9727..6f3fbc5 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -332,6 +332,7 @@ int rfcomm_connect_ind(struct rfcomm_session *s, u8 channel,
/* rfcomm_dev.status bit definitions */
#define RFCOMM_DEV_RELEASED 0
+#define RFCOMM_TTY_OWNED 1
struct rfcomm_dev_req {
s16 dev_id;
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index d9d4bc8..bb570d9 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -441,7 +441,7 @@ static int rfcomm_release_dev(void __user *arg)
tty_kref_put(tty);
}
- if (!test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags))
+ if (!test_bit(RFCOMM_TTY_OWNED, &dev->status))
tty_port_put(&dev->port);
tty_port_put(&dev->port);
@@ -685,8 +685,10 @@ static int rfcomm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
* when the last process closes the tty. The behaviour is expected by
* userspace.
*/
- if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags))
+ if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags)) {
+ set_bit(RFCOMM_TTY_OWNED, &dev->status);
tty_port_put(&dev->port);
+ }
return 0;
}
--
1.8.1.2
^ permalink raw reply related
* [PATCH 09/24] Bluetooth: Fix RFCOMM tty teardown race
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
RFCOMM tty device teardown can race with new tty device registration
for the same device id:
CPU 0 | CPU 1
rfcomm_dev_add | rfcomm_dev_destruct
| spin_lock
| list_del <== dev_id no longer used
| spin_unlock
spin_lock | .
[search rfcomm_dev_list] | .
[dev_id not in use] | .
[initialize new rfcomm_dev] | .
spin_unlock | .
| .
tty_port_register_device | tty_unregister_device
Don't remove rfcomm_dev from the device list until after tty device
unregistration has completed.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
net/bluetooth/rfcomm/tty.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index bb570d9..6ea08b0 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -84,10 +84,6 @@ static void rfcomm_dev_destruct(struct tty_port *port)
BT_DBG("dev %p dlc %p", dev, dlc);
- spin_lock(&rfcomm_dev_lock);
- list_del(&dev->list);
- spin_unlock(&rfcomm_dev_lock);
-
rfcomm_dlc_lock(dlc);
/* Detach DLC if it's owned by this dev */
if (dlc->owner == dev)
@@ -98,6 +94,10 @@ static void rfcomm_dev_destruct(struct tty_port *port)
tty_unregister_device(rfcomm_tty_driver, dev->id);
+ spin_lock(&rfcomm_dev_lock);
+ list_del(&dev->list);
+ spin_unlock(&rfcomm_dev_lock);
+
kfree(dev);
/* It's safe to call module_put() here because socket still
--
1.8.1.2
^ permalink raw reply related
* [PATCH 10/24] Bluetooth: Verify dlci not in use before rfcomm_dev create
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
Only one session/channel combination may be in use at any one
time. However, the failure does not occur until the tty is
opened (in rfcomm_dlc_open()).
Because these settings are actually bound at rfcomm device
creation (via RFCOMMCREATEDEV ioctl), validate and fail before
creating the rfcomm tty device.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
include/net/bluetooth/rfcomm.h | 1 +
net/bluetooth/rfcomm/core.c | 26 +++++++++++++++++++++++++-
net/bluetooth/rfcomm/tty.c | 8 ++++++++
3 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 6f3fbc5..79bb913 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -241,6 +241,7 @@ int rfcomm_dlc_send(struct rfcomm_dlc *d, struct sk_buff *skb);
int rfcomm_dlc_set_modem_status(struct rfcomm_dlc *d, u8 v24_sig);
int rfcomm_dlc_get_modem_status(struct rfcomm_dlc *d, u8 *v24_sig);
void rfcomm_dlc_accept(struct rfcomm_dlc *d);
+struct rfcomm_dlc *rfcomm_dlc_exists(bdaddr_t *src, bdaddr_t *dst, u8 channel);
#define rfcomm_dlc_lock(d) spin_lock(&d->lock)
#define rfcomm_dlc_unlock(d) spin_unlock(&d->lock)
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index facd8a7..646b6ff 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -359,6 +359,11 @@ static struct rfcomm_dlc *rfcomm_dlc_get(struct rfcomm_session *s, u8 dlci)
return NULL;
}
+static int rfcomm_check_channel(u8 channel)
+{
+ return channel < 1 || channel > 30;
+}
+
static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst, u8 channel)
{
struct rfcomm_session *s;
@@ -368,7 +373,7 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst,
BT_DBG("dlc %p state %ld %pMR -> %pMR channel %d",
d, d->state, src, dst, channel);
- if (channel < 1 || channel > 30)
+ if (rfcomm_check_channel(channel))
return -EINVAL;
if (d->state != BT_OPEN && d->state != BT_CLOSED)
@@ -513,6 +518,25 @@ no_session:
return r;
}
+struct rfcomm_dlc *rfcomm_dlc_exists(bdaddr_t *src, bdaddr_t *dst, u8 channel)
+{
+ struct rfcomm_session *s;
+ struct rfcomm_dlc *dlc = NULL;
+ u8 dlci;
+
+ if (rfcomm_check_channel(channel))
+ return ERR_PTR(-EINVAL);
+
+ rfcomm_lock();
+ s = rfcomm_session_get(src, dst);
+ if (s) {
+ dlci = __dlci(!s->initiator, channel);
+ dlc = rfcomm_dlc_get(s, dlci);
+ }
+ rfcomm_unlock();
+ return dlc;
+}
+
int rfcomm_dlc_send(struct rfcomm_dlc *d, struct sk_buff *skb)
{
int len = skb->len;
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 6ea08b0..a58d693 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -385,6 +385,14 @@ static int rfcomm_create_dev(struct sock *sk, void __user *arg)
dlc = rfcomm_pi(sk)->dlc;
rfcomm_dlc_hold(dlc);
} else {
+ /* Validate the channel is unused */
+ dlc = rfcomm_dlc_exists(&req.src, &req.dst, req.channel);
+ if (IS_ERR(dlc))
+ return PTR_ERR(dlc);
+ else if (dlc) {
+ rfcomm_dlc_put(dlc);
+ return -EBUSY;
+ }
dlc = rfcomm_dlc_alloc(GFP_KERNEL);
if (!dlc)
return -ENOMEM;
--
1.8.1.2
^ permalink raw reply related
* [PATCH 11/24] Bluetooth: Simplify RFCOMM session state eval
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
Merge conditional test for BT_LISTEN session state into following
switch statement (which is functionally equivalent).
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
net/bluetooth/rfcomm/core.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 646b6ff..b5a3785 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1967,12 +1967,11 @@ static void rfcomm_process_sessions(void)
continue;
}
- if (s->state == BT_LISTEN) {
+ switch (s->state) {
+ case BT_LISTEN:
rfcomm_accept_connection(s);
continue;
- }
- switch (s->state) {
case BT_BOUND:
s = rfcomm_check_connection(s);
break;
--
1.8.1.2
^ permalink raw reply related
* [PATCH 12/24] Bluetooth: Refactor deferred setup test in rfcomm_dlc_close()
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
Prepare for directly closing dlc if the RFCOMM session has not
yet been started; refactor the deferred setup test for only those
dlc states to which the test applies. Retains functional
equivalence.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
net/bluetooth/rfcomm/core.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index b5a3785..797b7e1 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -442,11 +442,18 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
switch (d->state) {
case BT_CONNECT:
case BT_CONFIG:
+ case BT_OPEN:
+ case BT_CONNECT2:
if (test_and_clear_bit(RFCOMM_DEFER_SETUP, &d->flags)) {
set_bit(RFCOMM_AUTH_REJECT, &d->flags);
rfcomm_schedule();
- break;
+ return 0;
}
+ }
+
+ switch (d->state) {
+ case BT_CONNECT:
+ case BT_CONFIG:
/* Fall through */
case BT_CONNECTED:
@@ -460,15 +467,6 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
}
break;
- case BT_OPEN:
- case BT_CONNECT2:
- if (test_and_clear_bit(RFCOMM_DEFER_SETUP, &d->flags)) {
- set_bit(RFCOMM_AUTH_REJECT, &d->flags);
- rfcomm_schedule();
- break;
- }
- /* Fall through */
-
default:
rfcomm_dlc_clear_timer(d);
--
1.8.1.2
^ permalink raw reply related
* [PATCH 13/24] Bluetooth: Refactor dlc disconnect logic in rfcomm_dlc_close()
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
Prepare for directly closing dlc if the RFCOMM session has not
yet been started; refactor the dlc disconnect logic into a separate
local function, __rfcomm_dlc_disconn(). Retains functional
equivalence.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
net/bluetooth/rfcomm/core.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 797b7e1..88f0edc 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -430,6 +430,20 @@ int rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst, u8 chann
return r;
}
+static void __rfcomm_dlc_disconn(struct rfcomm_dlc *d)
+{
+ struct rfcomm_session *s = d->session;
+
+ d->state = BT_DISCONN;
+ if (skb_queue_empty(&d->tx_queue)) {
+ rfcomm_send_disc(s, d->dlci);
+ rfcomm_dlc_set_timer(d, RFCOMM_DISC_TIMEOUT);
+ } else {
+ rfcomm_queue_disc(d);
+ rfcomm_dlc_set_timer(d, RFCOMM_DISC_TIMEOUT * 2);
+ }
+}
+
static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
{
struct rfcomm_session *s = d->session;
@@ -457,14 +471,7 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
/* Fall through */
case BT_CONNECTED:
- d->state = BT_DISCONN;
- if (skb_queue_empty(&d->tx_queue)) {
- rfcomm_send_disc(s, d->dlci);
- rfcomm_dlc_set_timer(d, RFCOMM_DISC_TIMEOUT);
- } else {
- rfcomm_queue_disc(d);
- rfcomm_dlc_set_timer(d, RFCOMM_DISC_TIMEOUT * 2);
- }
+ __rfcomm_dlc_disconn(d);
break;
default:
--
1.8.1.2
^ permalink raw reply related
* [PATCH 14/24] Bluetooth: Directly close dlc for not yet started RFCOMM session
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
If the RFCOMM session has not yet been started (ie., session is
still in BT_BOUND state) when a dlc is closed, directly close and
unlink the dlc rather than sending a DISC frame that is never
sent.
This allows the dlci to be immediately reused rather than waiting
for a 20 second timeout.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
net/bluetooth/rfcomm/core.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 88f0edc..8e14df6 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -467,13 +467,19 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
switch (d->state) {
case BT_CONNECT:
- case BT_CONFIG:
- /* Fall through */
-
case BT_CONNECTED:
__rfcomm_dlc_disconn(d);
break;
+ case BT_CONFIG:
+ if (s->state != BT_BOUND) {
+ __rfcomm_dlc_disconn(d);
+ break;
+ }
+ /* if closing a dlc in a session that hasn't been started,
+ * just close and unlink the dlc
+ */
+
default:
rfcomm_dlc_clear_timer(d);
--
1.8.1.2
^ permalink raw reply related
* [PATCH 15/24] Bluetooth: Fix unsafe RFCOMM device parenting
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
Accessing the results of hci_conn_hash_lookup_ba() is unsafe without
holding the hci_dev_lock() during the lookup. For example:
CPU 0 | CPU 1
hci_conn_hash_lookup_ba | hci_conn_del
rcu_read_lock | hci_conn_hash_del
list_for_each_entry_rcu | list_del_rcu
if (.....) | synchronize_rcu
rcu_read_unlock |
| hci_conn_del_sysfs
| hci_dev_put
| hci_conn_put
| put_device (last reference)
| bt_link_release
| kfree(conn)
return p << just freed |
Even if a hci_conn reference were taken (via hci_conn_get), would
not guarantee the lifetime of the sysfs device, but only safe
access to the in-memory structure.
Ensure the hci_conn device stays valid while the rfcomm device
is reparented; rename rfcomm_get_device() to rfcomm_reparent_device()
and perform the reparenting within the function while holding the
hci_dev_lock.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
net/bluetooth/rfcomm/tty.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index a58d693..2975bc4 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -167,20 +167,29 @@ static struct rfcomm_dev *rfcomm_dev_get(int id)
return dev;
}
-static struct device *rfcomm_get_device(struct rfcomm_dev *dev)
+static void rfcomm_reparent_device(struct rfcomm_dev *dev)
{
struct hci_dev *hdev;
struct hci_conn *conn;
hdev = hci_get_route(&dev->dst, &dev->src);
if (!hdev)
- return NULL;
+ return;
+ /* The lookup results are unsafe to access without the
+ * hci device lock (FIXME: why is this not documented?)
+ */
+ hci_dev_lock(hdev);
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &dev->dst);
- hci_dev_put(hdev);
+ /* Just because the acl link is in the hash table is no
+ * guarantee the sysfs device has been added ...
+ */
+ if (conn && device_is_registered(&conn->dev))
+ device_move(dev->tty_dev, &conn->dev, DPM_ORDER_DEV_AFTER_PARENT);
- return conn ? &conn->dev : NULL;
+ hci_dev_unlock(hdev);
+ hci_dev_put(hdev);
}
static ssize_t show_address(struct device *tty_dev, struct device_attribute *attr, char *buf)
@@ -589,8 +598,7 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
dev->err = err;
if (dlc->state == BT_CONNECTED) {
- device_move(dev->tty_dev, rfcomm_get_device(dev),
- DPM_ORDER_DEV_AFTER_PARENT);
+ rfcomm_reparent_device(dev);
wake_up_interruptible(&dev->port.open_wait);
} else if (dlc->state == BT_CLOSED)
--
1.8.1.2
^ permalink raw reply related
* [PATCH 16/24] Bluetooth: Fix RFCOMM parent device for reused dlc
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
The RFCOMM tty device is parented to the acl link device when
the dlc state_change(BT_CONNECTED) notification is received.
However, if the dlc from the RFCOMM socket is being reused
(RFCOMM_REUSE_DLC is set), then the dlc may already be connected,
and no notification will occur.
Instead, always parent the RFCOMM tty device to the acl link
device at registration time. If the acl link device is not available
(eg, because the dlc is not connected) then the tty will remain
unparented until the BT_CONNECTED notification is received.
Fixes regression with ModemManager when the rfcomm device is
created with the flag RFCOMM_REUSE_DLC.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
net/bluetooth/rfcomm/tty.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 2975bc4..fa1226f 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -316,6 +316,7 @@ out:
goto free;
}
+ rfcomm_reparent_device(dev);
dev_set_drvdata(dev->tty_dev, dev);
if (device_create_file(dev->tty_dev, &dev_attr_address) < 0)
--
1.8.1.2
^ permalink raw reply related
* [PATCH 17/24] Bluetooth: Rename __rfcomm_dev_get() to __rfcomm_dev_lookup()
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
Functions which search lists for matching id's are more
commonly named *_lookup, which is the convention in the
bluetooth core as well.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
net/bluetooth/rfcomm/tty.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index fa1226f..4a38b54 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -140,7 +140,7 @@ static const struct tty_port_operations rfcomm_port_ops = {
.carrier_raised = rfcomm_dev_carrier_raised,
};
-static struct rfcomm_dev *__rfcomm_dev_get(int id)
+static struct rfcomm_dev *__rfcomm_dev_lookup(int id)
{
struct rfcomm_dev *dev;
@@ -157,7 +157,7 @@ static struct rfcomm_dev *rfcomm_dev_get(int id)
spin_lock(&rfcomm_dev_lock);
- dev = __rfcomm_dev_get(id);
+ dev = __rfcomm_dev_lookup(id);
if (dev && !tty_port_get(&dev->port))
dev = NULL;
--
1.8.1.2
^ permalink raw reply related
* [PATCH 18/24] Bluetooth: Serialize RFCOMMCREATEDEV and RFCOMMRELEASEDEV ioctls
From: Peter Hurley @ 2014-02-10 1:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>
At least two different race conditions exist with multiple concurrent
RFCOMMCREATEDEV and RFCOMMRELEASEDEV ioctls:
* Multiple concurrent RFCOMMCREATEDEVs with RFCOMM_REUSE_DLC can
mistakenly share the same DLC.
* RFCOMMRELEASEDEV can destruct the rfcomm_dev still being
constructed by RFCOMMCREATEDEV.
Introduce rfcomm_ioctl_mutex to serialize these add/remove operations.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
net/bluetooth/rfcomm/tty.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 4a38b54..ef27695 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -40,6 +40,7 @@
#define RFCOMM_TTY_MAJOR 216 /* device node major id of the usb/bluetooth.c driver */
#define RFCOMM_TTY_MINOR 0
+static DEFINE_MUTEX(rfcomm_ioctl_mutex);
static struct tty_driver *rfcomm_tty_driver;
struct rfcomm_dev {
@@ -373,7 +374,7 @@ static struct sk_buff *rfcomm_wmalloc(struct rfcomm_dev *dev, unsigned long size
#define NOCAP_FLAGS ((1 << RFCOMM_REUSE_DLC) | (1 << RFCOMM_RELEASE_ONHUP))
-static int rfcomm_create_dev(struct sock *sk, void __user *arg)
+static int __rfcomm_create_dev(struct sock *sk, void __user *arg)
{
struct rfcomm_dev_req req;
struct rfcomm_dlc *dlc;
@@ -423,7 +424,7 @@ static int rfcomm_create_dev(struct sock *sk, void __user *arg)
return id;
}
-static int rfcomm_release_dev(void __user *arg)
+static int __rfcomm_release_dev(void __user *arg)
{
struct rfcomm_dev_req req;
struct rfcomm_dev *dev;
@@ -466,6 +467,28 @@ static int rfcomm_release_dev(void __user *arg)
return 0;
}
+static int rfcomm_create_dev(struct sock *sk, void __user *arg)
+{
+ int ret;
+
+ mutex_lock(&rfcomm_ioctl_mutex);
+ ret = __rfcomm_create_dev(sk, arg);
+ mutex_unlock(&rfcomm_ioctl_mutex);
+
+ return ret;
+}
+
+static int rfcomm_release_dev(void __user *arg)
+{
+ int ret;
+
+ mutex_lock(&rfcomm_ioctl_mutex);
+ ret = __rfcomm_release_dev(arg);
+ mutex_unlock(&rfcomm_ioctl_mutex);
+
+ return ret;
+}
+
static int rfcomm_get_dev_list(void __user *arg)
{
struct rfcomm_dev *dev;
--
1.8.1.2
^ 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