On 25/10/18 02:36, Spencer E. Olson wrote:
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@xxxxxxxxx> --- 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; + } +
Something you could do here is work out which insn->n in the instruction list is the largest, and allocate the 'data' once outside the instruction list handling loop instead of allocating it inside the loop.
-- -=( Ian Abbott <abbotti@xxxxxxxxx> || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address: )=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
