mirrors/alsa-lib
1
0
Fork 0
mirror of https://github.com/alsa-project/alsa-lib.git synced 2025-11-07 13:30:07 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions

pcm: Fix the wrong PCM object passed for locking/unlocking

Browse source 
Most of PCM API functions have snd_pcm_lock()/unlock() wraps for the
actual calls of ops, and some plugins try to unlock/relock internally
for the given PCM object.  This, unfortunately, causes a problem in
some configurations and leads to the unexpected behavior or deadlock.

The main problem is that we call snd_pcm_lock() with the given PCM
object, while calling the ops with pcm->op_arg or pcm->fast_op_arg as
the slave PCM object.  Meanwhile the plugin code assumes that the
passed PCM object is already locked, and calls snd_pcm_unlock().
This bug doesn't hit always because in most cases pcm->op_arg and
fast_op_arg are identical with pcm itself.  But in some configurations
they have different values, so the problem surfaces.

This patch is an attempt to resolve these inconsistencies.  It
replaces most of snd_pcm_lock()/unlock() calls with either pcm->op_arg
or pcm->fast_op_arg, depending on the call pattern, so that the plugin
code can safely run snd_pcm_unlock() to the given PCM object.

Fixes: 54931e5a54 ("pcm: Add thread-safety to PCM API")
Reported-by: Ben Russell <thematrixeatsyou@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This commit is contained in:
Takashi Iwai 2019-09-24 12:10:41 +02:00
parent 0732e47e6b
commit df483d3fe5
1 changed files with 57 additions and 57 deletions
Download patch file Download diff file Expand all files Collapse all files

114
src/pcm/pcm.c
View file

@ -789,7 +789,7 @@ int snd_pcm_nonblock(snd_pcm_t *pcm, int nonblock)
/* FIXME: __snd_pcm_lock() call below is commented out because of the /* FIXME: __snd_pcm_lock() call below is commented out because of the
* the possible deadlock in signal handler calling snd_pcm_abort() * the possible deadlock in signal handler calling snd_pcm_abort()
*/ */
/* __snd_pcm_lock(pcm); */ /* forced lock due to pcm field change */ /* __snd_pcm_lock(pcm->op_arg); */ /* forced lock due to pcm field change */
if (pcm->ops->nonblock) if (pcm->ops->nonblock)
err = pcm->ops->nonblock(pcm->op_arg, nonblock); err = pcm->ops->nonblock(pcm->op_arg, nonblock);
else else
@ -809,7 +809,7 @@ int snd_pcm_nonblock(snd_pcm_t *pcm, int nonblock)
pcm->mode &= ~SND_PCM_NONBLOCK; pcm->mode &= ~SND_PCM_NONBLOCK;
} }
unlock: unlock:
/* __snd_pcm_unlock(pcm); */ /* FIXME: see above */ /* __snd_pcm_unlock(pcm->op_arg); */ /* FIXME: see above */
return err; return err;
} }
@ -989,13 +989,13 @@ int snd_pcm_sw_params(snd_pcm_t *pcm, snd_pcm_sw_params_t *params)
return -EINVAL; return -EINVAL;
} }
#endif #endif
__snd_pcm_lock(pcm); /* forced lock due to pcm field change */ __snd_pcm_lock(pcm->op_arg); /* forced lock due to pcm field change */
if (pcm->ops->sw_params) if (pcm->ops->sw_params)
err = pcm->ops->sw_params(pcm->op_arg, params); err = pcm->ops->sw_params(pcm->op_arg, params);
else else
err = -ENOSYS; err = -ENOSYS;
if (err < 0) { if (err < 0) {
__snd_pcm_unlock(pcm); __snd_pcm_unlock(pcm->op_arg);
return err; return err;
} }
pcm->tstamp_mode = params->tstamp_mode; pcm->tstamp_mode = params->tstamp_mode;
@ -1008,7 +1008,7 @@ int snd_pcm_sw_params(snd_pcm_t *pcm, snd_pcm_sw_params_t *params)
pcm->silence_threshold = params->silence_threshold; pcm->silence_threshold = params->silence_threshold;
pcm->silence_size = params->silence_size; pcm->silence_size = params->silence_size;
pcm->boundary = params->boundary; pcm->boundary = params->boundary;
__snd_pcm_unlock(pcm); __snd_pcm_unlock(pcm->op_arg);
return 0; return 0;
} }
@ -1025,12 +1025,12 @@ int snd_pcm_status(snd_pcm_t *pcm, snd_pcm_status_t *status)
int err; int err;
assert(pcm && status); assert(pcm && status);
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
if (pcm->fast_ops->status) if (pcm->fast_ops->status)
err = pcm->fast_ops->status(pcm->fast_op_arg, status); err = pcm->fast_ops->status(pcm->fast_op_arg, status);
else else
err = -ENOSYS; err = -ENOSYS;
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return err; return err;
} }
@ -1050,9 +1050,9 @@ snd_pcm_state_t snd_pcm_state(snd_pcm_t *pcm)
snd_pcm_state_t state; snd_pcm_state_t state;
assert(pcm); assert(pcm);
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
state = __snd_pcm_state(pcm); state = __snd_pcm_state(pcm);
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return state; return state;
} }
@ -1078,9 +1078,9 @@ int snd_pcm_hwsync(snd_pcm_t *pcm)
SNDMSG("PCM not set up"); SNDMSG("PCM not set up");
return -EIO; return -EIO;
} }
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
err = __snd_pcm_hwsync(pcm); err = __snd_pcm_hwsync(pcm);
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return err; return err;
} }
@ -1123,9 +1123,9 @@ int snd_pcm_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delayp)
SNDMSG("PCM not set up"); SNDMSG("PCM not set up");
return -EIO; return -EIO;
} }
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
err = __snd_pcm_delay(pcm, delayp); err = __snd_pcm_delay(pcm, delayp);
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return err; return err;
} }
@ -1181,12 +1181,12 @@ int snd_pcm_htimestamp(snd_pcm_t *pcm, snd_pcm_uframes_t *avail, snd_htimestamp_
SNDMSG("PCM not set up"); SNDMSG("PCM not set up");
return -EIO; return -EIO;
} }
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
if (pcm->fast_ops->htimestamp) if (pcm->fast_ops->htimestamp)
err = pcm->fast_ops->htimestamp(pcm->fast_op_arg, avail, tstamp); err = pcm->fast_ops->htimestamp(pcm->fast_op_arg, avail, tstamp);
else else
err = -ENOSYS; err = -ENOSYS;
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return err; return err;
} }
@ -1209,12 +1209,12 @@ int snd_pcm_prepare(snd_pcm_t *pcm)
err = bad_pcm_state(pcm, ~P_STATE(DISCONNECTED)); err = bad_pcm_state(pcm, ~P_STATE(DISCONNECTED));
if (err < 0) if (err < 0)
return err; return err;
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
if (pcm->fast_ops->prepare) if (pcm->fast_ops->prepare)
err = pcm->fast_ops->prepare(pcm->fast_op_arg); err = pcm->fast_ops->prepare(pcm->fast_op_arg);
else else
err = -ENOSYS; err = -ENOSYS;
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return err; return err;
} }
@ -1236,12 +1236,12 @@ int snd_pcm_reset(snd_pcm_t *pcm)
SNDMSG("PCM not set up"); SNDMSG("PCM not set up");
return -EIO; return -EIO;
} }
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
if (pcm->fast_ops->reset) if (pcm->fast_ops->reset)
err = pcm->fast_ops->reset(pcm->fast_op_arg); err = pcm->fast_ops->reset(pcm->fast_op_arg);
else else
err = -ENOSYS; err = -ENOSYS;
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return err; return err;
} }
@ -1264,9 +1264,9 @@ int snd_pcm_start(snd_pcm_t *pcm)
err = bad_pcm_state(pcm, P_STATE(PREPARED)); err = bad_pcm_state(pcm, P_STATE(PREPARED));
if (err < 0) if (err < 0)
return err; return err;
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
err = __snd_pcm_start(pcm); err = __snd_pcm_start(pcm);
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return err; return err;
} }
@ -1296,12 +1296,12 @@ int snd_pcm_drop(snd_pcm_t *pcm)
P_STATE(SUSPENDED)); P_STATE(SUSPENDED));
if (err < 0) if (err < 0)
return err; return err;
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
if (pcm->fast_ops->drop) if (pcm->fast_ops->drop)
err = pcm->fast_ops->drop(pcm->fast_op_arg); err = pcm->fast_ops->drop(pcm->fast_op_arg);
else else
err = -ENOSYS; err = -ENOSYS;
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return err; return err;
} }
@ -1364,12 +1364,12 @@ int snd_pcm_pause(snd_pcm_t *pcm, int enable)
err = bad_pcm_state(pcm, P_STATE_RUNNABLE); err = bad_pcm_state(pcm, P_STATE_RUNNABLE);
if (err < 0) if (err < 0)
return err; return err;
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
if (pcm->fast_ops->pause) if (pcm->fast_ops->pause)
err = pcm->fast_ops->pause(pcm->fast_op_arg, enable); err = pcm->fast_ops->pause(pcm->fast_op_arg, enable);
else else
err = -ENOSYS; err = -ENOSYS;
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return err; return err;
} }
@ -1397,12 +1397,12 @@ snd_pcm_sframes_t snd_pcm_rewindable(snd_pcm_t *pcm)
err = bad_pcm_state(pcm, P_STATE_RUNNABLE); err = bad_pcm_state(pcm, P_STATE_RUNNABLE);
if (err < 0) if (err < 0)
return err; return err;
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
if (pcm->fast_ops->rewindable) if (pcm->fast_ops->rewindable)
result = pcm->fast_ops->rewindable(pcm->fast_op_arg); result = pcm->fast_ops->rewindable(pcm->fast_op_arg);
else else
result = -ENOSYS; result = -ENOSYS;
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return result; return result;
} }
@ -1430,12 +1430,12 @@ snd_pcm_sframes_t snd_pcm_rewind(snd_pcm_t *pcm, snd_pcm_uframes_t frames)
err = bad_pcm_state(pcm, P_STATE_RUNNABLE); err = bad_pcm_state(pcm, P_STATE_RUNNABLE);
if (err < 0) if (err < 0)
return err; return err;
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
if (pcm->fast_ops->rewind) if (pcm->fast_ops->rewind)
result = pcm->fast_ops->rewind(pcm->fast_op_arg, frames); result = pcm->fast_ops->rewind(pcm->fast_op_arg, frames);
else else
result = -ENOSYS; result = -ENOSYS;
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return result; return result;
} }
@ -1463,12 +1463,12 @@ snd_pcm_sframes_t snd_pcm_forwardable(snd_pcm_t *pcm)
err = bad_pcm_state(pcm, P_STATE_RUNNABLE); err = bad_pcm_state(pcm, P_STATE_RUNNABLE);
if (err < 0) if (err < 0)
return err; return err;
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
if (pcm->fast_ops->forwardable) if (pcm->fast_ops->forwardable)
result = pcm->fast_ops->forwardable(pcm->fast_op_arg); result = pcm->fast_ops->forwardable(pcm->fast_op_arg);
else else
result = -ENOSYS; result = -ENOSYS;
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return result; return result;
} }
@ -1500,12 +1500,12 @@ snd_pcm_sframes_t snd_pcm_forward(snd_pcm_t *pcm, snd_pcm_uframes_t frames)
err = bad_pcm_state(pcm, P_STATE_RUNNABLE); err = bad_pcm_state(pcm, P_STATE_RUNNABLE);
if (err < 0) if (err < 0)
return err; return err;
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
if (pcm->fast_ops->forward) if (pcm->fast_ops->forward)
result = pcm->fast_ops->forward(pcm->fast_op_arg, frames); result = pcm->fast_ops->forward(pcm->fast_op_arg, frames);
else else
result = -ENOSYS; result = -ENOSYS;
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return result; return result;
} }
use_default_symbol_version(__snd_pcm_forward, snd_pcm_forward, ALSA_0.9.0rc8); use_default_symbol_version(__snd_pcm_forward, snd_pcm_forward, ALSA_0.9.0rc8);
@ -1724,9 +1724,9 @@ int snd_pcm_poll_descriptors_count(snd_pcm_t *pcm)
int count; int count;
assert(pcm); assert(pcm);
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
count = __snd_pcm_poll_descriptors_count(pcm); count = __snd_pcm_poll_descriptors_count(pcm);
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return count; return count;
} }
@ -1782,9 +1782,9 @@ int snd_pcm_poll_descriptors(snd_pcm_t *pcm, struct pollfd *pfds, unsigned int s
int err; int err;
assert(pcm && pfds); assert(pcm && pfds);
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
err = __snd_pcm_poll_descriptors(pcm, pfds, space); err = __snd_pcm_poll_descriptors(pcm, pfds, space);
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return err; return err;
} }
@ -1817,9 +1817,9 @@ int snd_pcm_poll_descriptors_revents(snd_pcm_t *pcm, struct pollfd *pfds, unsign
int err; int err;
assert(pcm && pfds && revents); assert(pcm && pfds && revents);
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
err = __snd_pcm_poll_revents(pcm, pfds, nfds, revents); err = __snd_pcm_poll_revents(pcm, pfds, nfds, revents);
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return err; return err;
} }
@ -2816,9 +2816,9 @@ int snd_pcm_wait(snd_pcm_t *pcm, int timeout)
{ {
int err; int err;
__snd_pcm_lock(pcm); /* forced lock */ __snd_pcm_lock(pcm->fast_op_arg); /* forced lock */
err = __snd_pcm_wait_in_lock(pcm, timeout); err = __snd_pcm_wait_in_lock(pcm, timeout);
__snd_pcm_unlock(pcm); __snd_pcm_unlock(pcm->fast_op_arg);
return err; return err;
} }
@ -2865,9 +2865,9 @@ int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout)
return -EIO; return -EIO;
} }
do { do {
__snd_pcm_unlock(pcm); __snd_pcm_unlock(pcm->fast_op_arg);
err_poll = poll(pfd, npfds, timeout); err_poll = poll(pfd, npfds, timeout);
__snd_pcm_lock(pcm); __snd_pcm_lock(pcm->fast_op_arg);
if (err_poll < 0) { if (err_poll < 0) {
if (errno == EINTR && !PCMINABORT(pcm)) if (errno == EINTR && !PCMINABORT(pcm))
continue; continue;
@ -2926,9 +2926,9 @@ snd_pcm_sframes_t snd_pcm_avail_update(snd_pcm_t *pcm)
{ {
snd_pcm_sframes_t result; snd_pcm_sframes_t result;
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
result = __snd_pcm_avail_update(pcm); result = __snd_pcm_avail_update(pcm);
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return result; return result;
} }
@ -2956,13 +2956,13 @@ snd_pcm_sframes_t snd_pcm_avail(snd_pcm_t *pcm)
SNDMSG("PCM not set up"); SNDMSG("PCM not set up");
return -EIO; return -EIO;
} }
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
err = __snd_pcm_hwsync(pcm); err = __snd_pcm_hwsync(pcm);
if (err < 0) if (err < 0)
result = err; result = err;
else else
result = __snd_pcm_avail_update(pcm); result = __snd_pcm_avail_update(pcm);
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return result; return result;
} }
@ -2989,7 +2989,7 @@ int snd_pcm_avail_delay(snd_pcm_t *pcm,
SNDMSG("PCM not set up"); SNDMSG("PCM not set up");
return -EIO; return -EIO;
} }
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
err = __snd_pcm_hwsync(pcm); err = __snd_pcm_hwsync(pcm);
if (err < 0) if (err < 0)
goto unlock; goto unlock;
@ -3004,7 +3004,7 @@ int snd_pcm_avail_delay(snd_pcm_t *pcm,
*availp = sf; *availp = sf;
err = 0; err = 0;
unlock: unlock:
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return err; return err;
} }
@ -7192,9 +7192,9 @@ int snd_pcm_mmap_begin(snd_pcm_t *pcm,
err = bad_pcm_state(pcm, P_STATE_RUNNABLE); err = bad_pcm_state(pcm, P_STATE_RUNNABLE);
if (err < 0) if (err < 0)
return err; return err;
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
err = __snd_pcm_mmap_begin(pcm, areas, offset, frames); err = __snd_pcm_mmap_begin(pcm, areas, offset, frames);
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return err; return err;
} }
@ -7297,9 +7297,9 @@ snd_pcm_sframes_t snd_pcm_mmap_commit(snd_pcm_t *pcm,
err = bad_pcm_state(pcm, P_STATE_RUNNABLE); err = bad_pcm_state(pcm, P_STATE_RUNNABLE);
if (err < 0) if (err < 0)
return err; return err;
snd_pcm_lock(pcm); snd_pcm_lock(pcm->fast_op_arg);
result = __snd_pcm_mmap_commit(pcm, offset, frames); result = __snd_pcm_mmap_commit(pcm, offset, frames);
snd_pcm_unlock(pcm); snd_pcm_unlock(pcm->fast_op_arg);
return result; return result;
} }
@ -7375,7 +7375,7 @@ snd_pcm_sframes_t snd_pcm_read_areas(snd_pcm_t *pcm, const snd_pcm_channel_area_
if (size == 0) if (size == 0)
return 0; return 0;
__snd_pcm_lock(pcm); /* forced lock */ __snd_pcm_lock(pcm->fast_op_arg); /* forced lock */
while (size > 0) { while (size > 0) {
snd_pcm_uframes_t frames; snd_pcm_uframes_t frames;
snd_pcm_sframes_t avail; snd_pcm_sframes_t avail;
@ -7434,7 +7434,7 @@ snd_pcm_sframes_t snd_pcm_read_areas(snd_pcm_t *pcm, const snd_pcm_channel_area_
xfer += frames; xfer += frames;
} }
_end: _end:
__snd_pcm_unlock(pcm); __snd_pcm_unlock(pcm->fast_op_arg);
return xfer > 0 ? (snd_pcm_sframes_t) xfer : snd_pcm_check_error(pcm, err); return xfer > 0 ? (snd_pcm_sframes_t) xfer : snd_pcm_check_error(pcm, err);
} }
@ -7449,7 +7449,7 @@ snd_pcm_sframes_t snd_pcm_write_areas(snd_pcm_t *pcm, const snd_pcm_channel_area
if (size == 0) if (size == 0)
return 0; return 0;
__snd_pcm_lock(pcm); /* forced lock */ __snd_pcm_lock(pcm->fast_op_arg); /* forced lock */
while (size > 0) { while (size > 0) {
snd_pcm_uframes_t frames; snd_pcm_uframes_t frames;
snd_pcm_sframes_t avail; snd_pcm_sframes_t avail;
@ -7523,7 +7523,7 @@ snd_pcm_sframes_t snd_pcm_write_areas(snd_pcm_t *pcm, const snd_pcm_channel_area
xfer += frames; xfer += frames;
} }
_end: _end:
__snd_pcm_unlock(pcm); __snd_pcm_unlock(pcm->fast_op_arg);
return xfer > 0 ? (snd_pcm_sframes_t) xfer : snd_pcm_check_error(pcm, err); return xfer > 0 ? (snd_pcm_sframes_t) xfer : snd_pcm_check_error(pcm, err);
} }