remoteproc: add mutex protection for traces

In rproc_put() the buffers used to read remoteproc's traces were being unmapped
without any protection.  If at the same time a cat on the traces is done, it
may access an unmapped buffer leading to a crash.

This patch protects the access and unmapping of dump/trace buffers with a
mutex.  It also fixes some checkpatch issues in the trace functions.

Change-Id: I0a6f52e9df691bc1d4ff894febb9a20819b4b7c3

Signed-off-by: Miguel Vadillo <vadillo@ti.com>
Signed-off-by: Iliyan Malchev <malchev@google.com>

1 files changed, 56 insertions, 27 deletions

diff --git a/drivers/remoteproc/remoteproc.c b/drivers/remoteproc/remoteproc.c
index 5d2fa6c9495..12dd4ddc4e0 100644
--- a/drivers/remoteproc/remoteproc.c
+++ b/drivers/remoteproc/remoteproc.c
@@ -48,22 +48,30 @@ static DEFINE_SPINLOCK(rprocs_lock);
 /* debugfs parent dir */
 static struct dentry *rproc_dbg;
 
-static ssize_t rproc_format_trace_buf(char __user *userbuf, size_t count,
-				    loff_t *ppos, const void *src, int size)
+static ssize_t rproc_format_trace_buf(struct rproc *rproc, char __user *userbuf,
+					size_t count, loff_t *ppos,
+					const void *src, int size)
 {
 	const char *buf = (const char *) src;
 	ssize_t num_copied = 0;
 	static int from_beg;
 	loff_t pos = *ppos;
 	int *w_idx;
-	int i, w_pos;
+	int i, w_pos, ret = 0;
+
+	if (mutex_lock_interruptible(&rproc->tlock))
+		return -EINTR;
 
 	/* When src is NULL, the remoteproc is offline. */
-	if (!src)
-		return -EIO;
+	if (!src) {
+		ret = -EIO;
+		goto unlock;
+	}
 
-	if (size < 2 * sizeof(u32))
-		return -EINVAL;
+	if (size < 2 * sizeof(u32)) {
+		ret = -EINVAL;
+		goto unlock;
+	}
 
 	/* Assume write_idx is the penultimate byte in the buffer trace*/
 	size = size - (sizeof(u32) * 2);
@@ -76,25 +84,33 @@ static ssize_t rproc_format_trace_buf(char __user *userbuf, size_t count,
 	if (pos == 0)
 		*ppos = w_pos;
 
-	for (i = w_pos; i < size && buf[i]; i++);
+	for (i = w_pos; i < size && buf[i]; i++)
+		;
 
 	if (i > w_pos)
-		num_copied = simple_read_from_buffer(userbuf, count, ppos, src, i);
+		num_copied =
+			simple_read_from_buffer(userbuf, count, ppos, src, i);
 		if (!num_copied) {
 			from_beg = 1;
 			*ppos = 0;
-		} else
-			return num_copied;
+		} else {
+			ret = num_copied;
+			goto unlock;
+		}
 print_beg:
-	for (i = 0; i < w_pos && buf[i]; i++);
+	for (i = 0; i < w_pos && buf[i]; i++)
+		;
 
 	if (i) {
-		num_copied = simple_read_from_buffer(userbuf, count, ppos, src, i);
+		num_copied =
+			simple_read_from_buffer(userbuf, count, ppos, src, i);
 		if (!num_copied)
 			from_beg = 0;
-		return num_copied;
+		ret = num_copied;
 	}
-	return 0;
+unlock:
+	mutex_unlock(&rproc->tlock);
+	return ret;
 }
 
 static ssize_t rproc_name_read(struct file *filp, char __user *userbuf,
@@ -131,12 +147,12 @@ static int rproc_open_generic(struct inode *inode, struct file *file)
 	return 0;
 }
 
-#define DEBUGFS_READONLY_FILE(name, value, len)				\
+#define DEBUGFS_READONLY_FILE(name, v, l)				\
 static ssize_t name## _rproc_read(struct file *filp,			\
-		char __user *userbuf, size_t count, loff_t *ppos)	\
+		char __user *ubuf, size_t count, loff_t *ppos)		\
 {									\
 	struct rproc *rproc = filp->private_data;			\
-	return rproc_format_trace_buf(userbuf, count, ppos, value, len);\
+	return rproc_format_trace_buf(rproc, ubuf, count, ppos, v, l);	\
 }									\
 									\
 static const struct file_operations name ##_rproc_ops = {		\
@@ -911,10 +927,14 @@ static int rproc_handle_resources(struct rproc *rproc, struct fw_resource *rsc,
 	 * trace buffer memory _is_ normal memory, so we cast away the
 	 * __iomem to make sparse happy
 	 */
+
+	if (mutex_lock_interruptible(&rproc->tlock))
+		goto error;
+
 	if (trace_da0) {
 		ret = rproc_da_to_pa(rproc->memory_maps, trace_da0, &pa);
 		if (ret)
-			goto error;
+			goto unlock;
 		rproc->trace_buf0 = (__force void *)
 				ioremap_nocache(pa, rproc->trace_len0);
 		if (rproc->trace_buf0) {
@@ -925,20 +945,20 @@ static int rproc_handle_resources(struct rproc *rproc, struct fw_resource *rsc,
 							GFP_KERNEL);
 				if (!rproc->last_trace_buf0) {
 					ret = -ENOMEM;
-					goto error;
+					goto unlock;
 				}
 				DEBUGFS_ADD(trace0_last);
 			}
 		} else {
 			dev_err(dev, "can't ioremap trace buffer0\n");
 			ret = -EIO;
-			goto error;
+			goto unlock;
 		}
 	}
 	if (trace_da1) {
 		ret = rproc_da_to_pa(rproc->memory_maps, trace_da1, &pa);
 		if (ret)
-			goto error;
+			goto unlock;
 		rproc->trace_buf1 = (__force void *)
 				ioremap_nocache(pa, rproc->trace_len1);
 		if (rproc->trace_buf1) {
@@ -949,13 +969,14 @@ static int rproc_handle_resources(struct rproc *rproc, struct fw_resource *rsc,
 							GFP_KERNEL);
 				if (!rproc->last_trace_buf1) {
 					ret = -ENOMEM;
-					goto error;
+					goto unlock;
 				}
 				DEBUGFS_ADD(trace1_last);
 			}
 		} else {
 			dev_err(dev, "can't ioremap trace buffer1\n");
 			ret = -EIO;
+			goto unlock;
 		}
 	}
 
@@ -969,7 +990,7 @@ static int rproc_handle_resources(struct rproc *rproc, struct fw_resource *rsc,
 	if (cdump_da0) {
 		ret = rproc_da_to_pa(rproc->memory_maps, cdump_da0, &pa);
 		if (ret)
-			goto error;
+			goto unlock;
 		rproc->cdump_buf0 = (__force void *)
 					ioremap_nocache(pa, rproc->cdump_len0);
 		if (rproc->cdump_buf0)
@@ -977,13 +998,13 @@ static int rproc_handle_resources(struct rproc *rproc, struct fw_resource *rsc,
 		else {
 			dev_err(dev, "can't ioremap cdump buffer0\n");
 			ret = -EIO;
-			goto error;
+			goto unlock;
 		}
 	}
 	if (cdump_da1) {
 		ret = rproc_da_to_pa(rproc->memory_maps, cdump_da1, &pa);
 		if (ret)
-			goto error;
+			goto unlock;
 		rproc->cdump_buf1 = (__force void *)
 					ioremap_nocache(pa, rproc->cdump_len1);
 		if (rproc->cdump_buf1)
@@ -991,10 +1012,12 @@ static int rproc_handle_resources(struct rproc *rproc, struct fw_resource *rsc,
 		else {
 			dev_err(dev, "can't ioremap cdump buffer1\n");
 			ret = -EIO;
-			goto error;
 		}
 	}
 
+unlock:
+	mutex_unlock(&rproc->tlock);
+
 error:
 	if (ret && rproc->dbg_dir) {
 		debugfs_remove_recursive(rproc->dbg_dir);
@@ -1315,6 +1338,9 @@ void rproc_put(struct rproc *rproc)
 	if (--rproc->count)
 		goto out;
 
+	if (mutex_lock_interruptible(&rproc->tlock))
+		goto out;
+
 	if (rproc->trace_buf0)
 		/* iounmap normal memory, so make sparse happy */
 		iounmap((__force void __iomem *) rproc->trace_buf0);
@@ -1331,6 +1357,8 @@ void rproc_put(struct rproc *rproc)
 		iounmap((__force void __iomem *) rproc->cdump_buf1);
 	rproc->cdump_buf0 = rproc->cdump_buf1 = NULL;
 
+	mutex_unlock(&rproc->tlock);
+
 	rproc_reset_poolmem(rproc);
 	memset(rproc->memory_maps, 0, sizeof(rproc->memory_maps));
 	kfree(rproc->header);
@@ -1680,6 +1708,7 @@ int rproc_register(struct device *dev, const char *name,
 #endif
 	mutex_init(&rproc->lock);
 	mutex_init(&rproc->secure_lock);
+	mutex_init(&rproc->tlock);
 	INIT_WORK(&rproc->error_work, rproc_error_work);
 	BLOCKING_INIT_NOTIFIER_HEAD(&rproc->nbh);