From 140e28b83c4a31831cbf293d9cab20c603821202 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 23 Nov 2011 08:20:45 +0000 Subject: [PATCH] staging: line6: alloc/free buffers in hw_params/hw_free It is unsafe to free buffers in line6_pcm_stop(), which is not allowed to sleep, since urbs cannot be killed completely there and only unlinked. This means I/O may still be in progress and the URB completion function still gets invoked. This may result in memory corruption when buffer_in is freed but I/O is still pending. Additionally, line6_pcm_start() is not supposed to sleep so it should not use kmalloc(GFP_KERNEL). These issues can be resolved by performing buffer allocation/freeing in the .hw_params/.hw_free callbacks instead. The ALSA documentation also recommends doing buffer allocation/freeing in these callbacks. Signed-off-by: Stefan Hajnoczi Signed-off-by: Greg Kroah-Hartman --- drivers/staging/line6/capture.c | 15 +++++++++++++++ drivers/staging/line6/pcm.c | 24 ------------------------ drivers/staging/line6/playback.c | 15 +++++++++++++++ 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/drivers/staging/line6/capture.c b/drivers/staging/line6/capture.c index 9647154a492..d9da7edf1e7 100644 --- a/drivers/staging/line6/capture.c +++ b/drivers/staging/line6/capture.c @@ -9,6 +9,7 @@ * */ +#include #include #include #include @@ -319,6 +320,15 @@ static int snd_line6_capture_hw_params(struct snd_pcm_substream *substream, } /* -- [FD] end */ + line6pcm->buffer_in = kmalloc(LINE6_ISO_BUFFERS * LINE6_ISO_PACKETS * + line6pcm->max_packet_size, GFP_KERNEL); + + if (!line6pcm->buffer_in) { + dev_err(line6pcm->line6->ifcdev, + "cannot malloc capture buffer\n"); + return -ENOMEM; + } + ret = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(hw_params)); if (ret < 0) @@ -331,6 +341,11 @@ static int snd_line6_capture_hw_params(struct snd_pcm_substream *substream, /* hw_free capture callback */ static int snd_line6_capture_hw_free(struct snd_pcm_substream *substream) { + struct snd_line6_pcm *line6pcm = snd_pcm_substream_chip(substream); + + line6_unlink_wait_clear_audio_in_urbs(line6pcm); + kfree(line6pcm->buffer_in); + line6pcm->buffer_in = NULL; return snd_pcm_lib_free_pages(substream); } diff --git a/drivers/staging/line6/pcm.c b/drivers/staging/line6/pcm.c index ae984344749..2e4e164e81d 100644 --- a/drivers/staging/line6/pcm.c +++ b/drivers/staging/line6/pcm.c @@ -119,16 +119,6 @@ int line6_pcm_start(struct snd_line6_pcm *line6pcm, int channels) if (line6pcm->active_urb_in | line6pcm->unlink_urb_in) return -EBUSY; - line6pcm->buffer_in = - kmalloc(LINE6_ISO_BUFFERS * LINE6_ISO_PACKETS * - line6pcm->max_packet_size, GFP_KERNEL); - - if (!line6pcm->buffer_in) { - dev_err(line6pcm->line6->ifcdev, - "cannot malloc capture buffer\n"); - return -ENOMEM; - } - line6pcm->count_in = 0; line6pcm->prev_fsize = 0; err = line6_submit_audio_in_all_urbs(line6pcm); @@ -147,16 +137,6 @@ int line6_pcm_start(struct snd_line6_pcm *line6pcm, int channels) if (line6pcm->active_urb_out | line6pcm->unlink_urb_out) return -EBUSY; - line6pcm->buffer_out = - kmalloc(LINE6_ISO_BUFFERS * LINE6_ISO_PACKETS * - line6pcm->max_packet_size, GFP_KERNEL); - - if (!line6pcm->buffer_out) { - dev_err(line6pcm->line6->ifcdev, - "cannot malloc playback buffer\n"); - return -ENOMEM; - } - line6pcm->count_out = 0; err = line6_submit_audio_out_all_urbs(line6pcm); @@ -178,15 +158,11 @@ int line6_pcm_stop(struct snd_line6_pcm *line6pcm, int channels) if (((flags_old & MASK_CAPTURE) != 0) && ((flags_new & MASK_CAPTURE) == 0)) { line6_unlink_audio_in_urbs(line6pcm); - kfree(line6pcm->buffer_in); - line6pcm->buffer_in = NULL; } if (((flags_old & MASK_PLAYBACK) != 0) && ((flags_new & MASK_PLAYBACK) == 0)) { line6_unlink_audio_out_urbs(line6pcm); - kfree(line6pcm->buffer_out); - line6pcm->buffer_out = NULL; } #if LINE6_BACKUP_MONITOR_SIGNAL kfree(line6pcm->prev_fbuf); diff --git a/drivers/staging/line6/playback.c b/drivers/staging/line6/playback.c index 10c54383658..b3445274072 100644 --- a/drivers/staging/line6/playback.c +++ b/drivers/staging/line6/playback.c @@ -9,6 +9,7 @@ * */ +#include #include #include #include @@ -469,6 +470,15 @@ static int snd_line6_playback_hw_params(struct snd_pcm_substream *substream, } /* -- [FD] end */ + line6pcm->buffer_out = kmalloc(LINE6_ISO_BUFFERS * LINE6_ISO_PACKETS * + line6pcm->max_packet_size, GFP_KERNEL); + + if (!line6pcm->buffer_out) { + dev_err(line6pcm->line6->ifcdev, + "cannot malloc playback buffer\n"); + return -ENOMEM; + } + ret = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(hw_params)); if (ret < 0) @@ -481,6 +491,11 @@ static int snd_line6_playback_hw_params(struct snd_pcm_substream *substream, /* hw_free playback callback */ static int snd_line6_playback_hw_free(struct snd_pcm_substream *substream) { + struct snd_line6_pcm *line6pcm = snd_pcm_substream_chip(substream); + + line6_unlink_wait_clear_audio_out_urbs(line6pcm); + kfree(line6pcm->buffer_out); + line6pcm->buffer_out = NULL; return snd_pcm_lib_free_pages(substream); } -- 2.39.5