From a55de0f4931028622e2f6059007454758403f7c0 Mon Sep 17 00:00:00 2001 From: Ian Abbott Date: Fri, 23 Aug 2013 14:45:09 +0100 Subject: [PATCH] staging: comedi: comedi_bond: use krealloc() and fix memory leak `do_dev_config()` is called from the comedi 'attach' handler, `bonding_attach()`. The device private data structure contains a dynamically allocated array of pointers to "bonded" device structures which grows during the `do_dev_config()` call. The length of this array is in `devpriv->ndevs`. It currently uses a local function `realloc()` to allocate a new array, copy the old contents over and free the old array. It should be more efficient to use `krealloc()` as it may be able to use slack space at the end of the previous array and avoid a copy. The old `realloc()` function always freed the old buffer which meant that if it failed to allocate the new buffer it would lose the contents of the old buffer. Unfortunately, that contained pointers to more dynamically allocated memory, leading to a memory leak. If `krealloc()` fails, keep the old buffer and avoid the memory leak. The aforementioned pointers to more dynamically allocated memory will be cleaned up by the 'detach' handler, `bonding_detach()` which will be called by the comedi core as a consequence of `krealloc()` failing in `do_dev_config()`. Signed-off-by: Ian Abbott Signed-off-by: Greg Kroah-Hartman --- drivers/staging/comedi/drivers/comedi_bond.c | 28 +++++++------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/drivers/staging/comedi/drivers/comedi_bond.c b/drivers/staging/comedi/drivers/comedi_bond.c index 8e2696c85720..ccac7b9bc854 100644 --- a/drivers/staging/comedi/drivers/comedi_bond.c +++ b/drivers/staging/comedi/drivers/comedi_bond.c @@ -175,16 +175,6 @@ static int bonding_dio_insn_config(struct comedi_device *dev, return ret; } -static void *realloc(const void *oldmem, size_t newlen, size_t oldlen) -{ - void *newmem = kmalloc(newlen, GFP_KERNEL); - - if (newmem && oldmem) - memcpy(newmem, oldmem, min(oldlen, newlen)); - kfree(oldmem); - return newmem; -} - static int do_dev_config(struct comedi_device *dev, struct comedi_devconfig *it) { struct comedi_bond_private *devpriv = dev->private; @@ -201,8 +191,9 @@ static int do_dev_config(struct comedi_device *dev, struct comedi_devconfig *it) char file[sizeof("/dev/comediXXXXXX")]; int minor = it->options[i]; struct comedi_device *d; - int sdev = -1, nchans, tmp; - struct bonded_device *bdev = NULL; + int sdev = -1, nchans; + struct bonded_device *bdev; + struct bonded_device **devs; if (minor < 0 || minor >= COMEDI_NUM_BOARD_MINORS) { dev_err(dev->class_dev, @@ -257,17 +248,16 @@ static int do_dev_config(struct comedi_device *dev, struct comedi_devconfig *it) */ /* ergh.. ugly.. we need to realloc :( */ - tmp = devpriv->ndevs * sizeof(bdev); - devpriv->devs = - realloc(devpriv->devs, - ++devpriv->ndevs * sizeof(bdev), tmp); - if (!devpriv->devs) { + devs = krealloc(devpriv->devs, + (devpriv->ndevs + 1) * sizeof(*devs), + GFP_KERNEL); + if (!devs) { dev_err(dev->class_dev, "Could not allocate memory. Out of memory?\n"); return -ENOMEM; } - - devpriv->devs[devpriv->ndevs - 1] = bdev; + devpriv->devs = devs; + devpriv->devs[devpriv->ndevs++] = bdev; { /* Append dev:subdev to devpriv->name */ char buf[20]; -- 2.20.1