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 <stefanha@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff --git a/drivers/staging/line6/capture.c b/drivers/staging/line6/capture.c
index 9647154a4923157827210aa37b8f90ae8b2a14eb..d9da7edf1e7ac24c53a62d4953e896254ef8b9f6 100644 (file)
--- a/drivers/staging/line6/capture.c
+++ b/drivers/staging/line6/capture.c
@@ -9,6 +9,7 @@
  *
  */
 
+#include <linux/slab.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -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 ae984344749f034da61e83223e647fb394838716..2e4e164e81d8f93626fe70b4a683c5cfeee05e8a 100644 (file)
--- 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 10c54383658367ac4fc81eb2ddc6658a28e1da9d..b3445274072b616d003b9e54a61648c88ce84aa3 100644 (file)
--- a/drivers/staging/line6/playback.c
+++ b/drivers/staging/line6/playback.c
@@ -9,6 +9,7 @@
  *
  */
 
+#include <linux/slab.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -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);
 }