解放されたメモリにアクセスすることがある不具合を修正

driver:
ringbuffer: 関数の整理とロックの強化
px4: ringbufferの呼び出し関数を変更

[Thanks]: https://mevius.5ch.net/test/read.cgi/avi/1526812712/146

driver/px4.c

@@ -132,6 +132,20 @@ static int px4_init(struct px4_device *px4)
 		tsdev->lnb_power = false;
 		tsdev->px4 = px4;
 		atomic_set(&tsdev->streaming, 0);
+		ringbuffer_init(&tsdev->rgbuf);
+	}
+
+	return 0;
+}
+
+static int px4_term(struct px4_device *px4)
+{
+	int i;
+
+	for (i = 0; i < TSDEV_NUM; i++) {
+		struct px4_tsdev *tsdev = &px4->tsdev[i];
+
+		ringbuffer_term(&tsdev->rgbuf);
 	}
 
 	return 0;
@@ -765,7 +779,7 @@ static int px4_tsdev_start_streaming(struct px4_tsdev *tsdev)
 	if (ret)
 		goto fail;
 
-	ret = ringbuffer_init(&tsdev->rgbuf, buf_size);
+	ret = ringbuffer_alloc(&tsdev->rgbuf, buf_size);
 	if (ret)
 		goto fail;
 
@@ -780,8 +794,7 @@ static int px4_tsdev_start_streaming(struct px4_tsdev *tsdev)
 	return ret;
 
 fail_after_ringbuffer:
-	ringbuffer_flush(&tsdev->rgbuf);
-	ringbuffer_term(&tsdev->rgbuf);
+	ringbuffer_free(&tsdev->rgbuf);
 fail:
 	atomic_set(&tsdev->streaming, 0);
 	return ret;
@@ -803,8 +816,7 @@ static int px4_tsdev_stop_streaming(struct px4_tsdev *tsdev, bool avail)
 	if (!px4->streaming_count)
 		it930x_bus_stop_streaming(&px4->it930x.bus);
 
-	ringbuffer_flush(&tsdev->rgbuf);
-	ringbuffer_term(&tsdev->rgbuf);
+	ringbuffer_free(&tsdev->rgbuf);
 
 	if (!avail)
 		return 0;
@@ -1322,6 +1334,7 @@ static void px4_disconnect(struct usb_interface *intf)
 	}
 
 	// uninitialize
+	px4_term(px4);
 	it930x_bus_term(&px4->it930x.bus);
 	kfree(px4);
 

driver/ringbuffer.c

@@ -11,43 +11,100 @@
 
 #include "ringbuffer.h"
 
-int ringbuffer_init(struct ringbuffer *ringbuffer, size_t size)
+int ringbuffer_init(struct ringbuffer *ringbuffer)
 {
-	ringbuffer_term(ringbuffer);
-
-	ringbuffer->buf = (u8 *)__get_free_pages(GFP_KERNEL, get_order(size));
-	if (!ringbuffer->buf) {
-		return -ENOMEM;
-	}
-	ringbuffer->buf_size = size;
-	ringbuffer->data_size = 0;
-
 	spin_lock_init(&ringbuffer->lock);
+	atomic_set(&ringbuffer->avail, 0);
+	atomic_set(&ringbuffer->rw_cnt, 0);
+	atomic_set(&ringbuffer->wait_cnt, 0);
 	init_waitqueue_head(&ringbuffer->wait);
-	atomic_set(&ringbuffer->empty, 0);
-
-	ringbuffer->tail_pos = ringbuffer->head_pos = 0;
+	init_waitqueue_head(&ringbuffer->data_wait);
+	ringbuffer->buf = NULL;
+	ringbuffer->buf_size = 0;
+	ringbuffer->data_size = 0;
+	ringbuffer->tail_pos = 0;
+	ringbuffer->head_pos = 0;
 
 	return 0;
 }
 
 int ringbuffer_term(struct ringbuffer *ringbuffer)
 {
-	if (ringbuffer->buf) {
-		unsigned long p = (unsigned long)ringbuffer->buf;
-		ringbuffer->buf = NULL;
-		free_pages(p, get_order(ringbuffer->buf_size));
+	return ringbuffer_free(ringbuffer);
+}
+
+static void _ringbuffer_free(struct ringbuffer *ringbuffer)
+{
+	free_pages((unsigned long)ringbuffer->buf, get_order(ringbuffer->buf_size));
+
+	ringbuffer->buf = NULL;
+	ringbuffer->buf_size = 0;
+	ringbuffer->data_size = 0;
+	ringbuffer->tail_pos = 0;
+	ringbuffer->head_pos = 0;
+}
+
+int ringbuffer_alloc(struct ringbuffer *ringbuffer, size_t size)
+{
+	int ret = 0;
+
+	// Acquire lock
+	if (atomic_add_return(1, &ringbuffer->wait_cnt) != 1) {
+		// Someone is waiting
+		ret = -EAGAIN;
+		goto exit;
 	}
+	atomic_set(&ringbuffer->avail, 0);
+	wake_up(&ringbuffer->data_wait);
+	wait_event(ringbuffer->wait, !atomic_read(&ringbuffer->rw_cnt));
+
+	if (ringbuffer->buf)
+		_ringbuffer_free(ringbuffer);
+
+	// Allocate
+
+	ringbuffer->buf = (u8 *)__get_free_pages(GFP_KERNEL, get_order(size));
+	if (!ringbuffer->buf) {
+		ret = -ENOMEM;
+		goto exit;
+	}
+
+	ringbuffer->buf_size = size;
+	ringbuffer->data_size = 0;
+	ringbuffer->tail_pos = 0;
+	ringbuffer->head_pos = 0;
+
+	atomic_set(&ringbuffer->avail, 1);
+
+exit:
+	// Release lock
+	atomic_sub(1, &ringbuffer->wait_cnt);
 
 	return 0;
 }
 
-int ringbuffer_flush(struct ringbuffer *ringbuffer)
+int ringbuffer_free(struct ringbuffer *ringbuffer)
 {
-	ringbuffer->tail_pos = ringbuffer->head_pos = 0;
-	ringbuffer->data_size = 0;
-	atomic_set(&ringbuffer->empty, 1);
-	wake_up(&ringbuffer->wait);
+	int ret = 0;
+
+	if (!ringbuffer->buf)
+		return 0;
+
+	// Acquire lock
+	if (atomic_add_return(1, &ringbuffer->wait_cnt) != 1) {
+		// Someone is waiting
+		ret = -EAGAIN;
+		goto exit;
+	}
+	atomic_set(&ringbuffer->avail, 0);
+	wake_up(&ringbuffer->data_wait);
+	wait_event(ringbuffer->wait, !atomic_read(&ringbuffer->rw_cnt));
+
+	_ringbuffer_free(ringbuffer);
+
+exit:
+	// Release lock
+	atomic_sub(1, &ringbuffer->wait_cnt);
 
 	return 0;
 }
@@ -57,9 +114,12 @@ int ringbuffer_write(struct ringbuffer *ringbuffer, const void *data, size_t len
 	unsigned long flags;
 	const u8 *p = data;
 	size_t buf_size, data_size, tail_pos, write_size;
+	int rr;
 
-	if (!ringbuffer->buf)
-		return -EINVAL;
+	if (!atomic_read(&ringbuffer->avail) || atomic_read(&ringbuffer->wait_cnt))
+		return -EIO;
+
+	atomic_add(1, &ringbuffer->rw_cnt);
 
 	buf_size = ringbuffer->buf_size;
 	tail_pos = ringbuffer->tail_pos;
@@ -84,10 +144,15 @@ int ringbuffer_write(struct ringbuffer *ringbuffer, const void *data, size_t len
 
 		spin_lock_irqsave(&ringbuffer->lock, flags);
 		ringbuffer->data_size += write_size;
-		wake_up(&ringbuffer->wait);
 		spin_unlock_irqrestore(&ringbuffer->lock, flags);
+
+		wake_up(&ringbuffer->data_wait);
 	}
 
+	rr = atomic_sub_return(1, &ringbuffer->rw_cnt);
+	if (atomic_read(&ringbuffer->wait_cnt) && !rr)
+		wake_up(&ringbuffer->wait);
+
 	return 0;
 }
 
@@ -95,17 +160,20 @@ int ringbuffer_read_to_user(struct ringbuffer *ringbuffer, void __user *buf, siz
 {
 	u8 *p = buf;
 	size_t buf_size, l = *len, buf_pos = 0;
+	int rr;
+
+	if (!atomic_read(&ringbuffer->avail) || atomic_read(&ringbuffer->wait_cnt))
+		return -EIO;
+
+	atomic_add(1, &ringbuffer->rw_cnt);
 
 	buf_size = ringbuffer->buf_size;
 
-	while (l > buf_pos && !atomic_read(&ringbuffer->empty)) {
+	while (l > buf_pos && atomic_read(&ringbuffer->avail)) {
 		size_t data_size, head_pos, read_size, t;
 		unsigned long r;
 
-		if (!ringbuffer->buf)
-			break;
-
-		wait_event(ringbuffer->wait, (ringbuffer->data_size || atomic_read(&ringbuffer->empty)));
+		wait_event(ringbuffer->data_wait, (ringbuffer->data_size || !atomic_read(&ringbuffer->avail)));
 
 		spin_lock(&ringbuffer->lock);
 		data_size = ringbuffer->data_size;
@@ -144,5 +212,9 @@ int ringbuffer_read_to_user(struct ringbuffer *ringbuffer, void __user *buf, siz
 
 	*len = buf_pos;
 
+	rr = atomic_sub_return(1, &ringbuffer->rw_cnt);
+	if (atomic_read(&ringbuffer->wait_cnt) && !rr)
+		wake_up(&ringbuffer->wait);
+
 	return 0;
 }

driver/ringbuffer.h

@@ -10,8 +10,11 @@
 
 struct ringbuffer {
 	spinlock_t lock;	// for data_size
-	atomic_t empty;
+	atomic_t avail;
+	atomic_t rw_cnt;
+	atomic_t wait_cnt;
 	wait_queue_head_t wait;
+	wait_queue_head_t data_wait;
 	u8 *buf;
 	size_t buf_size;
 	size_t data_size;
@@ -19,9 +22,10 @@ struct ringbuffer {
 	size_t head_pos;	// read
 };
 
-int ringbuffer_init(struct ringbuffer *ringbuffer, size_t size);
+int ringbuffer_init(struct ringbuffer *ringbuffer);
 int ringbuffer_term(struct ringbuffer *ringbuffer);
-int ringbuffer_flush(struct ringbuffer *ringbuffer);
+int ringbuffer_alloc(struct ringbuffer *ringbuffer, size_t size);
+int ringbuffer_free(struct ringbuffer *ringbuffer);
 int ringbuffer_write(struct ringbuffer *ringbuffer, const void *data, size_t len);
 int ringbuffer_read_to_user(struct ringbuffer *ringbuffer, void __user *buf, size_t *len);