ALSA: pcm: Fix possible OOB access in PCM oss plugins [Linux 5.1]

ALSA: pcm: Fix possible OOB access in PCM oss plugins [Linux 5.1]

This Linux kernel change "ALSA: pcm: Fix possible OOB access in PCM oss plugins" is included in the Linux 5.1 release. This change is authored by Takashi Iwai <tiwai [at] suse.de> on Fri Mar 22 16:00:54 2019 +0100. The commit for this change in Linux stable tree is ca0214e (patch).

ALSA: pcm: Fix possible OOB access in PCM oss plugins

The PCM OSS emulation converts and transfers the data on the fly via
"plugins".  The data is converted over the dynamically allocated
buffer for each plugin, and recently syzkaller caught OOB in this
flow.

Although the bisection by syzbot pointed out to the commit
65766ee0bf7f ("ALSA: oss: Use kvzalloc() for local buffer
allocations"), this is merely a commit to replace vmalloc() with
kvmalloc(), hence it can't be the cause.  The further debug action
revealed that this happens in the case where a slave PCM doesn't
support only the stereo channels while the OSS stream is set up for a
mono channel.  Below is a brief explanation:

At each OSS parameter change, the driver sets up the PCM hw_params
again in snd_pcm_oss_change_params_lock().  This is also the place
where plugins are created and local buffers are allocated.  The
problem is that the plugins are created before the final hw_params is
determined.  Namely, two snd_pcm_hw_param_near() calls for setting the
period size and periods may influence on the final result of channels,
rates, etc, too, while the current code has already created plugins
beforehand with the premature values.  So, the plugin believes that
channels=1, while the actual I/O is with channels=2, which makes the
driver reading/writing over the allocated buffer size.

The fix is simply to move the plugin allocation code after the final
hw_params call.

Reported-by: syzbot+d4503ae45b65c5bc1194@syzkaller.appspotmail.com
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>

There are 43 lines of Linux source code added/deleted in this change. Code changes to Linux kernel are as follows.

 sound/core/oss/pcm_oss.c | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index d5b0d7b..f6ae680 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -940,6 +940,28 @@ static int snd_pcm_oss_change_params_locked(struct snd_pcm_substream *substream)
    oss_frame_size = snd_pcm_format_physical_width(params_format(params)) *
             params_channels(params) / 8;

+   err = snd_pcm_oss_period_size(substream, params, sparams);
+   if (err < 0)
+       goto failure;
+
+   n = snd_pcm_plug_slave_size(substream, runtime->oss.period_bytes / oss_frame_size);
+   err = snd_pcm_hw_param_near(substream, sparams, SNDRV_PCM_HW_PARAM_PERIOD_SIZE, n, NULL);
+   if (err < 0)
+       goto failure;
+
+   err = snd_pcm_hw_param_near(substream, sparams, SNDRV_PCM_HW_PARAM_PERIODS,
+                    runtime->oss.periods, NULL);
+   if (err < 0)
+       goto failure;
+
+   snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_DROP, NULL);
+
+   err = snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_HW_PARAMS, sparams);
+   if (err < 0) {
+       pcm_dbg(substream->pcm, "HW_PARAMS failed: %i\n", err);
+       goto failure;
+   }
+
 #ifdef CONFIG_SND_PCM_OSS_PLUGINS
    snd_pcm_oss_plugin_clear(substream);
    if (!direct) {
@@ -974,27 +996,6 @@ static int snd_pcm_oss_change_params_locked(struct snd_pcm_substream *substream)
    }
 #endif

-   err = snd_pcm_oss_period_size(substream, params, sparams);
-   if (err < 0)
-       goto failure;
-
-   n = snd_pcm_plug_slave_size(substream, runtime->oss.period_bytes / oss_frame_size);
-   err = snd_pcm_hw_param_near(substream, sparams, SNDRV_PCM_HW_PARAM_PERIOD_SIZE, n, NULL);
-   if (err < 0)
-       goto failure;
-
-   err = snd_pcm_hw_param_near(substream, sparams, SNDRV_PCM_HW_PARAM_PERIODS,
-                    runtime->oss.periods, NULL);
-   if (err < 0)
-       goto failure;
-
-   snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_DROP, NULL);
-
-   if ((err = snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_HW_PARAMS, sparams)) < 0) {
-       pcm_dbg(substream->pcm, "HW_PARAMS failed: %i\n", err);
-       goto failure;
-   }
-
    if (runtime->oss.trigger) {
        sw_params->start_threshold = 1;
    } else {

Leave a Reply

Your email address will not be published. Required fields are marked *