race conditions in alsa core

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



In the process of trying to discover that kind of locking ALSA provides to a
driver and what the driver must take care of itself, I think I have found some
race conditions inside the alsa core.

I think there are likely quite a few, but this example was one of the easiest
to prove.

ALSA uses unlocked_ioctl, which means ioctl calls aren't protected by the BKL
and may run at the same time.

Some ioctls, for instance HW_PARAMS, don't appear to have any locking done by
ALSA.  It's entirely possible for two hw_params calls to run at the same time
on the same substream, and the ALSA core (and probably many drivers too) can't
handle this.

The following patch helps to create a larger window for the race condition, so
that it can be triggered consistently.

--- a/sound/core/pcm_native.c   Tue Sep 04 09:02:06 2007 +0000
+++ b/sound/core/pcm_native.c   Sat Sep 22 17:12:37 2007 -0700
@@ -411,6 +411,7 @@ static int snd_pcm_hw_params(struct snd_
 	runtime->channels = params_channels(params);
 	runtime->rate = params_rate(params);
 	runtime->period_size = params_period_size(params);
+	{ static unsigned long x=1; if(test_and_change_bit(0, &x)) msleep(10); }
 	runtime->periods = params_periods(params);
 	runtime->buffer_size = params_buffer_size(params);
 	runtime->tick_time = params_tick_time(params);

This causes every other call to snd_pcm_hw_params() to have a 10ms delay
between setting the period size and period count.  If there are two calls
within 10ms, the period count should come from the first call (which slept),
while the period size will be from the second call.

My test program will call the HW_PARAMS ioctl twice from different threads
running at the same time.  The first call will ask for the maximum number of
periods with minimum size, the second call for the minimum number of periods
of maximum size.

When prepare is called, we discover that the runtime is set for the maximum
number of periods of maximum size, which exceeds the buffer size.  Of course
even if it didn't exceed the buffer size, the params should be from one call
or the other, not some random combination of both.

I've attaching a very simple dummy ALSA driver which shows this.  All it does
is print timestamps and a few parameters when the various callback functions
are invoked.  The output looks like this:

as_open - 0.084 us
as_hw_params - 552.864 us periods 128, period size 1024
as_hw_params - 589.996 us periods 1, period size 131072
as_prepare - 11065.307 us periods 128, period size 131072
as_close - 11278.229 us

Notice how in prepare the runtime has parameters that are a combination of the
two hw_params calls.  This test program should trigger the same race with any
other ALSA driver.  Just adjust the period count and size.

The test program doesn't use the ALSA lib.  It does need the headers from the
alsa lib package to compile, since this seems to be the only place where user
space headers for the device interface are provided.
/* Using ALSA with threads test program
   Copyright (C) 2007 Trent Piepho <xyzzy@xxxxxxxxxxxxx> */

/* To compile, provide the path to the alsa-lib include directory. e.g.
   gcc -O2 -Wall -Ialsa-lib/include -pthread test_hwparams.c
   This is necessary to get the types and defines for using the ALSA
   device directly.  The alsa kernel includes don't work from userspace. */


#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <signal.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/ioctl.h>
#include <sys/poll.h>
#include <fcntl.h>
#include <stdarg.h>
#include <limits.h>
#include <pthread.h>
#include "local.h"
#include "sound/asound.h"
#include "global.h"
#include "input.h"
#include "output.h"
#include "conf.h"
#include "pcm.h"

static const char *dev = "/dev/snd/pcmC1D0p";

struct hw_params_args {
    int fd;
    int periods;
    int period_size;
};

static void *hw_params(void *arg)
{
    struct hw_params_args *args = arg;
    snd_pcm_hw_params_t params = {.rmask = ~0U, .info = ~0U};
    struct _snd_interval *interval;
    int r;

    memset(params.masks, 0xff, SNDRV_MASK_MAX/8 *
           (SNDRV_PCM_HW_PARAM_LAST_MASK - SNDRV_PCM_HW_PARAM_FIRST_MASK + 1));
    for(r=0; r<=SNDRV_PCM_HW_PARAM_LAST_INTERVAL-SNDRV_PCM_HW_PARAM_FIRST_INTERVAL; r++)
	params.intervals[r].max = UINT_MAX;

    interval = params.intervals + (SNDRV_PCM_HW_PARAM_PERIOD_SIZE - SNDRV_PCM_HW_PARAM_FIRST_INTERVAL);
    interval->min = interval->max = args->period_size;
    interval = params.intervals + (SNDRV_PCM_HW_PARAM_PERIODS - SNDRV_PCM_HW_PARAM_FIRST_INTERVAL);
    interval->min = interval->max = args->periods;
    r = ioctl(args->fd, SNDRV_PCM_IOCTL_HW_PARAMS, &params);
    printf("hw_params = %d\n", r);
    return NULL;
}

static void *prepare(void *arg)
{
    int fd = (int)arg, r;

    r = ioctl(fd, SNDRV_PCM_IOCTL_PREPARE);
    printf("prepare = %d\n", r);
    return NULL;
}

int main(int argc, char *argv[])
{
    int fd;
    pthread_t thread;
    struct hw_params_args arg1, arg2;

    if(argc>1) dev = argv[1];

    fd = open(dev, O_RDWR);
    printf("fd = %d\n", fd);

    arg1.fd = arg2.fd = fd;
    arg1.period_size = 1024; arg1.periods = 128;
    arg2.period_size = 128*1024; arg2.periods = 1;
    pthread_create(&thread, NULL, hw_params, &arg1);
    hw_params(&arg2);
    pthread_join(thread, NULL);

    prepare((void*)fd);
    return 0;
}
/* Very simple dummy ALSA driver.
   Copyright (C) 2007 Trent Piepho <xyzzy@xxxxxxxxxxxxx> */

#include <linux/version.h>
#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,19)
#include <linux/config.h>
#endif
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/init.h>
#include <linux/sched.h>

#include <sound/driver.h>
#include <sound/core.h>
#include <sound/pcm.h>
#include <sound/pcm_params.h>
#include <sound/initval.h>

MODULE_LICENSE("GPL");

#include <linux/calc64.h>
#define MHZ	1544.426
#define INT	(unsigned int)(MHZ * (1<<12) + 0.5)
#define FRAC	(unsigned int)(MHZ * (1<<21) / 1000.0 + 0.5)
#define timestamp(str, args...)	{ u64 _time; u32 _rem; rdtscll(_time); \
	_time = (_time - chip->starttime)<<12; \
	_rem = do_div(_time, INT); \
	printk("%s - %u.%03u us " str "\n", __FUNCTION__, \
		(unsigned int)_time, (_rem << 9) / FRAC , ## args); }

struct snd_as {
	u64	starttime;
};

static struct snd_pcm_hardware as_hw = {
	.info = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
	        SNDRV_PCM_INFO_BLOCK_TRANSFER | SNDRV_PCM_INFO_INTERLEAVED,
	.formats = SNDRV_PCM_FMTBIT_U8,
	.rates = SNDRV_PCM_RATE_8000_48000,
	.rate_min = 8000,
	.rate_max = 48000,
	.channels_min = 1,
	.channels_max = 1,
	.buffer_bytes_max = (128 * 1024),
	.period_bytes_min = 1024,
	.period_bytes_max = (128 * 1024),
	.periods_min = 1,
	.periods_max = 128,
};

static int as_open(struct snd_pcm_substream *substream)
{
	struct snd_as *chip = snd_pcm_substream_chip(substream);
	struct snd_pcm_runtime *runtime = substream->runtime;
	int err;

	runtime->hw = as_hw;
	rdtscll(chip->starttime);
	err = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
	timestamp("");
	return err;
}

static int as_close(struct snd_pcm_substream *substream)
{
	struct snd_as *chip = snd_pcm_substream_chip(substream);

	timestamp("");
	return 0;
}

static int as_hw_params(struct snd_pcm_substream *substream, 
			struct snd_pcm_hw_params *hw_params)
{
	struct snd_as *chip = snd_pcm_substream_chip(substream);

	timestamp("periods %d, period size %d", params_periods(hw_params), 
	          params_period_size(hw_params));
	return 0;
}

static int as_prepare(struct snd_pcm_substream *substream)
{
	struct snd_as *chip = snd_pcm_substream_chip(substream);
	struct snd_pcm_runtime *runtime = substream->runtime;

	timestamp("periods %d, period size %lu", runtime->periods, runtime->period_size);
	return 0;
}

static int as_trigger(struct snd_pcm_substream *substream, int cmd)
{
	struct snd_as *chip = snd_pcm_substream_chip(substream);

	timestamp("cmd %d", cmd);
	return 0;
}

static snd_pcm_uframes_t as_pointer(struct snd_pcm_substream *substream)
{
	struct snd_as *chip = snd_pcm_substream_chip(substream);

	timestamp("");
	return 0;
}

static struct snd_pcm_ops as_pcm_ops = {
	.open = as_open,
	.close = as_close,
	.ioctl = snd_pcm_lib_ioctl,
	.hw_params = as_hw_params,
	.prepare = as_prepare,
	.trigger = as_trigger,
	.pointer = as_pointer,
};

static struct snd_card *card;

static int __init init(void)
{
	struct snd_as *chip;
	struct snd_pcm *pcm;
	int err;

	card = snd_card_new(-1, SNDRV_DEFAULT_STR1, THIS_MODULE, sizeof(*chip));
	chip = card->private_data;
	snd_pcm_new(card, "Test", 0, 1, 1, &pcm);
	pcm->private_data = chip;
	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &as_pcm_ops);
	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &as_pcm_ops);

	err = snd_card_register(card);
	return err;
}
module_init(init);

static void __exit exit(void)
{
        snd_card_free(card);
	return;
}
module_exit(exit);
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux