[PATCH] staging: comedi: change do_insn*_ioctl to allow more samples

Spencer E. Olson olsonse at umich.edu
Thu Oct 25 01:36:41 UTC 2018


Changes do_insn*_ioctl functions to allow for data lengths for each
comedi_insn of up to 2^16.  This patch also changes these functions to only
allocate as much memory as is necessary for each comedi_insn, rather than
allocating a fixed-sized scratch space.

In testing some user-space code for the new INSN_DEVICE_CONFIG_GET_ROUTES
facility with some newer hardware, I discovered that do_insn_ioctl and
do_insnlist_ioctl limited the amount of data that can be passed into the
kernel for insn's to a length of 256.  For some newer hardware, the number
of routes can be greater than 1000.  Working around the old limits (256)
would complicate the user-space/kernel interaction.

The new upper limit is reasonable with current memory available and does
not otherwise impact the memory footprint for any current or otherwise
typical configuration.

Signed-off-by: Spencer E. Olson <olsonse at umich.edu>
---
 drivers/staging/comedi/comedi_fops.c | 37 +++++++++++++++++-----------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index c1c6b2b4ab91..a163ec2df872 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1500,7 +1500,7 @@ static int parse_insn(struct comedi_device *dev, struct comedi_insn *insn,
  *	data (for reads) to insns[].data pointers
  */
 /* arbitrary limits */
-#define MAX_SAMPLES 256
+#define MAX_SAMPLES 65536
 static int do_insnlist_ioctl(struct comedi_device *dev,
 			     struct comedi_insnlist __user *arg, void *file)
 {
@@ -1513,12 +1513,6 @@ static int do_insnlist_ioctl(struct comedi_device *dev,
 	if (copy_from_user(&insnlist, arg, sizeof(insnlist)))
 		return -EFAULT;
 
-	data = kmalloc_array(MAX_SAMPLES, sizeof(unsigned int), GFP_KERNEL);
-	if (!data) {
-		ret = -ENOMEM;
-		goto error;
-	}
-
 	insns = kcalloc(insnlist.n_insns, sizeof(*insns), GFP_KERNEL);
 	if (!insns) {
 		ret = -ENOMEM;
@@ -1539,6 +1533,14 @@ static int do_insnlist_ioctl(struct comedi_device *dev,
 			ret = -EINVAL;
 			goto error;
 		}
+
+		data = kmalloc_array(insns[i].n, sizeof(unsigned int),
+				     GFP_KERNEL);
+		if (!data) {
+			ret = -ENOMEM;
+			goto error;
+		}
+
 		if (insns[i].insn & INSN_MASK_WRITE) {
 			if (copy_from_user(data, insns[i].data,
 					   insns[i].n * sizeof(unsigned int))) {
@@ -1560,6 +1562,11 @@ static int do_insnlist_ioctl(struct comedi_device *dev,
 				goto error;
 			}
 		}
+
+		/* free up the single-instruction data memory */
+		kfree(data);
+		data = NULL;
+
 		if (need_resched())
 			schedule();
 	}
@@ -1594,20 +1601,20 @@ static int do_insn_ioctl(struct comedi_device *dev,
 	unsigned int *data = NULL;
 	int ret = 0;
 
-	data = kmalloc_array(MAX_SAMPLES, sizeof(unsigned int), GFP_KERNEL);
-	if (!data) {
-		ret = -ENOMEM;
-		goto error;
-	}
-
 	if (copy_from_user(&insn, arg, sizeof(insn))) {
-		ret = -EFAULT;
-		goto error;
+		return -EFAULT;
 	}
 
 	/* This is where the behavior of insn and insnlist deviate. */
 	if (insn.n > MAX_SAMPLES)
 		insn.n = MAX_SAMPLES;
+
+	data = kmalloc_array(insn.n, sizeof(unsigned int), GFP_KERNEL);
+	if (!data) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
 	if (insn.insn & INSN_MASK_WRITE) {
 		if (copy_from_user(data,
 				   insn.data,
-- 
2.17.1



More information about the devel mailing list