dahdi: Protect echocan creation/destruction with mutex.

This closes a reference and memory leak when multiple CPUs are enabling echocan
on a single channel in parallel.

The essential problem is that the call to try_module_get() is not serialized.
Two separate threads can come into ioctl_echocan() on the same channel, they
coordinate via the dahdi_chan.lock to release any current echocan, but then both
create a new echocan state, bump the reference on the module, and the last one
through will actually attach the new state to the channel. The earlier reference
/ memory is leaked.

I tried to conceive of a way to fix this leak without adding a new lock, but the
choices where calling throught the function pointers with dahdi_chan.lock.
Otherwise I needed to change the semantics of echocan_create /free which would
ripple through the hardware echocan modules.

Signed-off-by: Shaun Ruffell <sruffell@digium.com>
Signed-off-by: Russ Meyerriecks <rmeyerriecks@digium.com>
This commit is contained in:
Shaun Ruffell 2014-06-19 16:40:01 -05:00 committed by Russ Meyerriecks
parent cb50ae1500
commit 761e02da52
2 changed files with 6 additions and 1 deletions

View File

@ -1884,6 +1884,7 @@ static void __dahdi_init_chan(struct dahdi_chan *chan)
might_sleep(); might_sleep();
spin_lock_init(&chan->lock); spin_lock_init(&chan->lock);
mutex_init(&chan->mutex);
init_waitqueue_head(&chan->waitq); init_waitqueue_head(&chan->waitq);
if (!chan->master) if (!chan->master)
chan->master = chan; chan->master = chan;
@ -6331,6 +6332,7 @@ ioctl_echocancel(struct dahdi_chan *chan, struct dahdi_echocanparams *ecp,
if (ecp->tap_length == 0) { if (ecp->tap_length == 0) {
/* disable mode, don't need to inspect params */ /* disable mode, don't need to inspect params */
mutex_lock(&chan->mutex);
spin_lock_irqsave(&chan->lock, flags); spin_lock_irqsave(&chan->lock, flags);
ec_state = chan->ec_state; ec_state = chan->ec_state;
chan->ec_state = NULL; chan->ec_state = NULL;
@ -6341,7 +6343,7 @@ ioctl_echocancel(struct dahdi_chan *chan, struct dahdi_echocanparams *ecp,
ec_state->ops->echocan_free(chan, ec_state); ec_state->ops->echocan_free(chan, ec_state);
release_echocan(ec_current); release_echocan(ec_current);
} }
mutex_unlock(&chan->mutex);
return 0; return 0;
} }
@ -6358,6 +6360,7 @@ ioctl_echocancel(struct dahdi_chan *chan, struct dahdi_echocanparams *ecp,
goto exit_with_free; goto exit_with_free;
} }
mutex_lock(&chan->mutex);
/* free any echocan that may be on the channel already */ /* free any echocan that may be on the channel already */
spin_lock_irqsave(&chan->lock, flags); spin_lock_irqsave(&chan->lock, flags);
ec_state = chan->ec_state; ec_state = chan->ec_state;
@ -6428,6 +6431,7 @@ ioctl_echocancel(struct dahdi_chan *chan, struct dahdi_echocanparams *ecp,
} }
exit_with_free: exit_with_free:
mutex_unlock(&chan->mutex);
kfree(params); kfree(params);
return ret; return ret;

View File

@ -425,6 +425,7 @@ struct dahdi_chan {
int lastnumbufs; int lastnumbufs;
#endif #endif
spinlock_t lock; spinlock_t lock;
struct mutex mutex;
char name[40]; char name[40];
/* Specified by DAHDI */ /* Specified by DAHDI */
/*! \brief DAHDI channel number */ /*! \brief DAHDI channel number */