Please consider the attached patch, also available at https://github.com/pabigot/rtl-sdr/tree/fix-libusb-segv. This reworks the buffer management in rtlsdr_read_async to ensure that libusb has no outstanding transactions related to transfer buffers prior to those buffers being freed.

Peter


>From 24f0ffd6f74c948d6838535af3e2418cc544dbac Mon Sep 17 00:00:00 2001
From: "Peter A. Bigot" <[email protected]>
Date: Sun, 13 Jul 2014 05:02:56 -0500
Subject: [PATCH] read_async: rework USB buffer management to avoid libusb API
 violation

rtl_sdr on 64-bit Ubuntu 12.04 generates segfaults when interrupted
because the buffer used by a transfer initiated in rtlsdr_read_async()
was freed before the corresponding callback had been processed.

This commit reworks the async buffer management so that the callback is
responsible for freeing the buffer during the cancellation process and
ensures that the async call does not return until all buffers have been
released.

Ref: http://lists.osmocom.org/pipermail/osmocom-sdr/2013-January/000440.html
Signed-off-by: Peter A. Bigot <[email protected]>
---
 src/librtlsdr.c | 47 ++++++++++++++++++++++-------------------------
 1 file changed, 22 insertions(+), 25 deletions(-)

diff --git a/src/librtlsdr.c b/src/librtlsdr.c
index 9a3ebcd..c509559 100644
--- a/src/librtlsdr.c
+++ b/src/librtlsdr.c
@@ -1648,8 +1648,11 @@ static void LIBUSB_CALL _libusb_callback(struct libusb_transfer *xfer)
 		if (dev->cb)
 			dev->cb(xfer->buffer, xfer->actual_length, dev->cb_ctx);
 
-		libusb_submit_transfer(xfer); /* resubmit transfer */
-		dev->xfer_errors = 0;
+		if (RTLSDR_RUNNING == dev->async_status) {
+			libusb_submit_transfer(xfer); /* resubmit transfer */
+			dev->xfer_errors = 0;
+			return;
+		}
 	} else if (LIBUSB_TRANSFER_CANCELLED != xfer->status) {
 #ifndef _WIN32
 		if (LIBUSB_TRANSFER_ERROR == xfer->status)
@@ -1666,6 +1669,18 @@ static void LIBUSB_CALL _libusb_callback(struct libusb_transfer *xfer)
 		}
 #endif
 	}
+	/* Cancelled or errored-out transfer: free the async buffer so
+	 * rtlsdr_read_async knows it's safe to quit */
+        if (dev->xfer) {
+		int i;
+		for (i = 0; i < dev->xfer_buf_num; ++i) {
+			if (xfer == dev->xfer[i]) {
+				libusb_free_transfer(xfer);
+				dev->xfer[i] = NULL;
+				break;
+			}
+		}
+        }
 }
 
 int rtlsdr_wait_async(rtlsdr_dev_t *dev, rtlsdr_read_async_cb_t cb, void *ctx)
@@ -1708,6 +1723,7 @@ static int _rtlsdr_free_async_buffers(rtlsdr_dev_t *dev)
 
 	if (dev->xfer) {
 		for(i = 0; i < dev->xfer_buf_num; ++i) {
+			/* Buffers normally freed and zeroed in callback */
 			if (dev->xfer[i]) {
 				libusb_free_transfer(dev->xfer[i]);
 			}
@@ -1781,7 +1797,8 @@ int rtlsdr_read_async(rtlsdr_dev_t *dev, rtlsdr_read_async_cb_t cb, void *ctx,
 		}
 	}
 
-	while (RTLSDR_INACTIVE != dev->async_status) {
+	next_status = dev->async_status;
+	while (RTLSDR_INACTIVE != next_status) {
 		r = libusb_handle_events_timeout_completed(dev->ctx, &tv,
 							   &dev->async_cancel);
 		if (r < 0) {
@@ -1801,28 +1818,8 @@ int rtlsdr_read_async(rtlsdr_dev_t *dev, rtlsdr_read_async_cb_t cb, void *ctx,
 				if (!dev->xfer[i])
 					continue;
 
-				if (LIBUSB_TRANSFER_CANCELLED !=
-						dev->xfer[i]->status) {
-					r = libusb_cancel_transfer(dev->xfer[i]);
-					/* handle events after canceling
-					 * to allow transfer status to
-					 * propagate */
-					libusb_handle_events_timeout_completed(dev->ctx,
-									       &zerotv, NULL);
-					if (r < 0)
-						continue;
-
-					next_status = RTLSDR_CANCELING;
-				}
-			}
-
-			if (dev->dev_lost || RTLSDR_INACTIVE == next_status) {
-				/* handle any events that still need to
-				 * be handled before exiting after we
-				 * just cancelled all transfers */
-				libusb_handle_events_timeout_completed(dev->ctx,
-								       &zerotv, NULL);
-				break;
+                                next_status = RTLSDR_CANCELING;
+				(void)libusb_cancel_transfer(dev->xfer[i]);
 			}
 		}
 	}
-- 
1.8.5.5

Reply via email to