All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ v2] gobex: Fix segfault caused by interrupted transfer
@ 2020-06-04 16:02 Denis Grigorev
  2020-06-04 16:31 ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 3+ messages in thread
From: Denis Grigorev @ 2020-06-04 16:02 UTC (permalink / raw)
  To: linux-bluetooth



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH BlueZ v2] gobex: Fix segfault caused by interrupted transfer
  2020-06-04 16:02 [PATCH BlueZ v2] gobex: Fix segfault caused by interrupted transfer Denis Grigorev
@ 2020-06-04 16:31 ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2020-06-04 16:31 UTC (permalink / raw)
  To: Denis Grigorev; +Cc: linux-bluetooth

Hi Denis,

Looks like you have sent an empty patch.

On Thu, Jun 4, 2020 at 9:14 AM Denis Grigorev <d.grigorev@omprussia.ru> wrote:
>
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH BlueZ v2] gobex: Fix segfault caused by interrupted transfer
@ 2020-06-05 11:18 Denis Grigorev
  0 siblings, 0 replies; 3+ messages in thread
From: Denis Grigorev @ 2020-06-05 11:18 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Denis Grigorev

When a obex transfer is interrupted by a peer in the middle, the response
G_OBEX_RSP_FORBIDDEN comes and the transfer is freed in transfer_complete.
However gobex is still ref'ed and gobex->io continues to be writable,
so write_data() and then g_obex_abort() are called. When the abort response
comes, struct obc_transfer is already freed, which leads to the crash.

Backtrace :
__GI___pthread_mutex_lock (mutex=0x65732f74) at pthread_mutex_lock.c:67
0xecc6eeda in dbus_connection_get_object_path_data () from /usr/lib/libdbus-1.so.3
0x000457d4 in g_dbus_emit_property_changed_full () at gdbus/object.c:1794
0x00045868 in g_dbus_emit_property_changed () at gdbus/object.c:1832
0x000367f0 in transfer_set_status () at obexd/client/transfer.c:211
0x0003681e in transfer_set_status () at obexd/client/transfer.c:206
xfer_complete () at obexd/client/transfer.c:672
0x00022df6 in transfer_complete () at gobex/gobex-transfer.c:103
0x00022f44 in transfer_abort_response () at gobex/gobex-transfer.c:124
0x00020a0e in handle_response () at gobex/gobex.c:1128
0x00020dde in incoming_data () at gobex/gobex.c:1373

This commit introduces g_obex_drop_tx_queue(), which will be called if
a transfer error detected. After the tx queue is dropped, obex shuts
down gracefully.

To be able to close L2CAP connection when there are pending frames,
Linux kernels below v4.3 must have following commits cherry-picked:

commit 2baea85dec1a ("Bluetooth: L2CAP ERTM shutdown protect sk and chan")
commit f65468f6e26c ("Bluetooth: Make __l2cap_wait_ack more efficient")
commit 451e4c6c6b3f ("Bluetooth: Add BT_DBG to l2cap_sock_shutdown()")
commit cb02a25583b5 ("Bluetooth: __l2cap_wait_ack() use msecs_to_jiffies()")
commit e432c72c464d ("Bluetooth: __l2cap_wait_ack() add defensive timeout")
commit e7456437c15a ("Bluetooth: Unwind l2cap_sock_shutdown()")
commit 04ba72e6b24f ("Bluetooth: Reorganize mutex lock in l2cap_sock_shutdown()")
commit 9f7378a9d6ce ("Bluetooth: l2cap_disconnection_req priority over shutdown")
---

Changes in v2:
- Edit commit message formatting.

 gobex/gobex-transfer.c |  5 +++++
 gobex/gobex.c          | 10 ++++++++++
 gobex/gobex.h          |  1 +
 3 files changed, 16 insertions(+)

diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c
index bc9930679..e96e61fbc 100644
--- a/gobex/gobex-transfer.c
+++ b/gobex/gobex-transfer.c
@@ -100,6 +100,11 @@ static void transfer_complete(struct transfer *transfer, GError *err)
 
 	g_obex_debug(G_OBEX_DEBUG_TRANSFER, "transfer %u", id);
 
+	if (err) {
+		/* No further tx must be performed */
+		g_obex_drop_tx_queue(transfer->obex);
+	}
+
 	transfer->complete_func(transfer->obex, err, transfer->user_data);
 	/* Check if the complete_func removed the transfer */
 	if (find_transfer(id) == NULL)
diff --git a/gobex/gobex.c b/gobex/gobex.c
index 77f1aaafd..d68a85eb6 100644
--- a/gobex/gobex.c
+++ b/gobex/gobex.c
@@ -521,6 +521,16 @@ static void enable_tx(GObex *obex)
 	obex->write_source = g_io_add_watch(obex->io, cond, write_data, obex);
 }
 
+void g_obex_drop_tx_queue(GObex *obex)
+{
+	struct pending_pkt *p;
+
+	g_obex_debug(G_OBEX_DEBUG_COMMAND, "");
+
+	while ((p = g_queue_pop_head(obex->tx_queue)))
+		pending_pkt_free(p);
+}
+
 static gboolean g_obex_send_internal(GObex *obex, struct pending_pkt *p,
 								GError **err)
 {
diff --git a/gobex/gobex.h b/gobex/gobex.h
index b223a2fac..a94d9246e 100644
--- a/gobex/gobex.h
+++ b/gobex/gobex.h
@@ -63,6 +63,7 @@ gboolean g_obex_remove_request_function(GObex *obex, guint id);
 void g_obex_suspend(GObex *obex);
 void g_obex_resume(GObex *obex);
 gboolean g_obex_srm_active(GObex *obex);
+void g_obex_drop_tx_queue(GObex *obex);
 
 GObex *g_obex_new(GIOChannel *io, GObexTransportType transport_type,
 						gssize rx_mtu, gssize tx_mtu);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-06-05 11:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-04 16:02 [PATCH BlueZ v2] gobex: Fix segfault caused by interrupted transfer Denis Grigorev
2020-06-04 16:31 ` Luiz Augusto von Dentz
2020-06-05 11:18 Denis Grigorev

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.