Commit a55de0f4 authored by Ian Abbott's avatar Ian Abbott Committed by Greg Kroah-Hartman

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: default avatarIan Abbott <abbotti@mev.co.uk>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 0f3ce1a6
...@@ -175,16 +175,6 @@ static int bonding_dio_insn_config(struct comedi_device *dev, ...@@ -175,16 +175,6 @@ static int bonding_dio_insn_config(struct comedi_device *dev,
return ret; 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) static int do_dev_config(struct comedi_device *dev, struct comedi_devconfig *it)
{ {
struct comedi_bond_private *devpriv = dev->private; struct comedi_bond_private *devpriv = dev->private;
...@@ -201,8 +191,9 @@ static int do_dev_config(struct comedi_device *dev, struct comedi_devconfig *it) ...@@ -201,8 +191,9 @@ static int do_dev_config(struct comedi_device *dev, struct comedi_devconfig *it)
char file[sizeof("/dev/comediXXXXXX")]; char file[sizeof("/dev/comediXXXXXX")];
int minor = it->options[i]; int minor = it->options[i];
struct comedi_device *d; struct comedi_device *d;
int sdev = -1, nchans, tmp; int sdev = -1, nchans;
struct bonded_device *bdev = NULL; struct bonded_device *bdev;
struct bonded_device **devs;
if (minor < 0 || minor >= COMEDI_NUM_BOARD_MINORS) { if (minor < 0 || minor >= COMEDI_NUM_BOARD_MINORS) {
dev_err(dev->class_dev, dev_err(dev->class_dev,
...@@ -257,17 +248,16 @@ static int do_dev_config(struct comedi_device *dev, struct comedi_devconfig *it) ...@@ -257,17 +248,16 @@ static int do_dev_config(struct comedi_device *dev, struct comedi_devconfig *it)
*/ */
/* ergh.. ugly.. we need to realloc :( */ /* ergh.. ugly.. we need to realloc :( */
tmp = devpriv->ndevs * sizeof(bdev); devs = krealloc(devpriv->devs,
devpriv->devs = (devpriv->ndevs + 1) * sizeof(*devs),
realloc(devpriv->devs, GFP_KERNEL);
++devpriv->ndevs * sizeof(bdev), tmp); if (!devs) {
if (!devpriv->devs) {
dev_err(dev->class_dev, dev_err(dev->class_dev,
"Could not allocate memory. Out of memory?\n"); "Could not allocate memory. Out of memory?\n");
return -ENOMEM; return -ENOMEM;
} }
devpriv->devs = devs;
devpriv->devs[devpriv->ndevs - 1] = bdev; devpriv->devs[devpriv->ndevs++] = bdev;
{ {
/* Append dev:subdev to devpriv->name */ /* Append dev:subdev to devpriv->name */
char buf[20]; char buf[20];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment