* [PATCH v2] ALSA: pcm: oss: Fix race at SNDCTL_DSP_SETTRIGGER
@ 2023-09-21 6:42 Ma Ke
2023-09-21 13:29 ` Takashi Iwai
2023-09-21 15:20 ` kernel test robot
0 siblings, 2 replies; 3+ messages in thread
From: Ma Ke @ 2023-09-21 6:42 UTC (permalink / raw)
To: perex, tiwai, Liam.Howlett, rppt, mgorman, mhocko, make_ruc2021,
surenb
Cc: alsa-devel, linux-kernel
There is a small race window at snd_pcm_oss_set_trigger() that is
called from OSS PCM SNDCTL_DSP_SETTRIGGER ioctl; namely the function
calls snd_pcm_oss_make_ready() at first, then takes the params_lock
mutex for the rest. When the stream is set up again by another thread
between them, it leads to inconsistency, and may result in unexpected
results such as NULL dereference of OSS buffer as a fuzzer spotted
recently.
The fix is simply to cover snd_pcm_oss_make_ready() call into the same
params_lock mutex with snd_pcm_oss_make_ready_locked() variant.
Signed-off-by: Ma Ke <make_ruc2021@163.com>
---
sound/core/oss/pcm_oss.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index 728c211142d1..f6340a2fe52b 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -2083,21 +2083,15 @@ static int snd_pcm_oss_set_trigger(struct snd_pcm_oss_file *pcm_oss_file, int tr
psubstream = pcm_oss_file->streams[SNDRV_PCM_STREAM_PLAYBACK];
csubstream = pcm_oss_file->streams[SNDRV_PCM_STREAM_CAPTURE];
- if (psubstream) {
- err = snd_pcm_oss_make_ready(psubstream);
- if (err < 0)
- return err;
- }
- if (csubstream) {
- err = snd_pcm_oss_make_ready(csubstream);
- if (err < 0)
- return err;
- }
if (psubstream) {
runtime = psubstream->runtime;
cmd = 0;
if (mutex_lock_interruptible(&runtime->oss.params_lock))
return -ERESTARTSYS;
+ err = snd_pcm_oss_make_ready_locked(psubstream);
+ if (err < 0)
+ mutex_unlock(&runtime->oss.params_lock);
+ return err;
if (trigger & PCM_ENABLE_OUTPUT) {
if (runtime->oss.trigger)
goto _skip1;
@@ -2128,6 +2122,10 @@ static int snd_pcm_oss_set_trigger(struct snd_pcm_oss_file *pcm_oss_file, int tr
cmd = 0;
if (mutex_lock_interruptible(&runtime->oss.params_lock))
return -ERESTARTSYS;
+ err = snd_pcm_oss_make_ready_locked(csubstream);
+ if (err < 0)
+ mutex_unlock(&runtime->oss.params_lock);
+ return err;
if (trigger & PCM_ENABLE_INPUT) {
if (runtime->oss.trigger)
goto _skip2;
--
2.37.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v2] ALSA: pcm: oss: Fix race at SNDCTL_DSP_SETTRIGGER
2023-09-21 6:42 [PATCH v2] ALSA: pcm: oss: Fix race at SNDCTL_DSP_SETTRIGGER Ma Ke
@ 2023-09-21 13:29 ` Takashi Iwai
2023-09-21 15:20 ` kernel test robot
1 sibling, 0 replies; 3+ messages in thread
From: Takashi Iwai @ 2023-09-21 13:29 UTC (permalink / raw)
To: Ma Ke
Cc: perex, tiwai, Liam.Howlett, rppt, mgorman, mhocko, surenb,
alsa-devel, linux-kernel
On Thu, 21 Sep 2023 08:42:58 +0200,
Ma Ke wrote:
>
> There is a small race window at snd_pcm_oss_set_trigger() that is
> called from OSS PCM SNDCTL_DSP_SETTRIGGER ioctl; namely the function
> calls snd_pcm_oss_make_ready() at first, then takes the params_lock
> mutex for the rest. When the stream is set up again by another thread
> between them, it leads to inconsistency, and may result in unexpected
> results such as NULL dereference of OSS buffer as a fuzzer spotted
> recently.
> The fix is simply to cover snd_pcm_oss_make_ready() call into the same
> params_lock mutex with snd_pcm_oss_make_ready_locked() variant.
>
> Signed-off-by: Ma Ke <make_ruc2021@163.com>
> ---
> sound/core/oss/pcm_oss.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
> index 728c211142d1..f6340a2fe52b 100644
> --- a/sound/core/oss/pcm_oss.c
> +++ b/sound/core/oss/pcm_oss.c
> @@ -2083,21 +2083,15 @@ static int snd_pcm_oss_set_trigger(struct snd_pcm_oss_file *pcm_oss_file, int tr
> psubstream = pcm_oss_file->streams[SNDRV_PCM_STREAM_PLAYBACK];
> csubstream = pcm_oss_file->streams[SNDRV_PCM_STREAM_CAPTURE];
>
> - if (psubstream) {
> - err = snd_pcm_oss_make_ready(psubstream);
> - if (err < 0)
> - return err;
> - }
> - if (csubstream) {
> - err = snd_pcm_oss_make_ready(csubstream);
> - if (err < 0)
> - return err;
> - }
> if (psubstream) {
> runtime = psubstream->runtime;
> cmd = 0;
> if (mutex_lock_interruptible(&runtime->oss.params_lock))
> return -ERESTARTSYS;
> + err = snd_pcm_oss_make_ready_locked(psubstream);
> + if (err < 0)
> + mutex_unlock(&runtime->oss.params_lock);
> + return err;
This breaks totally; you missed braces... (Ditto for another place).
Takashi
> if (trigger & PCM_ENABLE_OUTPUT) {
> if (runtime->oss.trigger)
> goto _skip1;
> @@ -2128,6 +2122,10 @@ static int snd_pcm_oss_set_trigger(struct snd_pcm_oss_file *pcm_oss_file, int tr
> cmd = 0;
> if (mutex_lock_interruptible(&runtime->oss.params_lock))
> return -ERESTARTSYS;
> + err = snd_pcm_oss_make_ready_locked(csubstream);
> + if (err < 0)
> + mutex_unlock(&runtime->oss.params_lock);
> + return err;
> if (trigger & PCM_ENABLE_INPUT) {
> if (runtime->oss.trigger)
> goto _skip2;
> --
> 2.37.2
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] ALSA: pcm: oss: Fix race at SNDCTL_DSP_SETTRIGGER
2023-09-21 6:42 [PATCH v2] ALSA: pcm: oss: Fix race at SNDCTL_DSP_SETTRIGGER Ma Ke
2023-09-21 13:29 ` Takashi Iwai
@ 2023-09-21 15:20 ` kernel test robot
1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2023-09-21 15:20 UTC (permalink / raw)
To: Ma Ke, perex, tiwai, Liam.Howlett, rppt, mgorman, mhocko, surenb
Cc: oe-kbuild-all, alsa-devel, linux-kernel
Hi Ma,
kernel test robot noticed the following build warnings:
[auto build test WARNING on tiwai-sound/for-next]
[also build test WARNING on tiwai-sound/for-linus linus/master v6.6-rc2 next-20230921]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ma-Ke/ALSA-pcm-oss-Fix-race-at-SNDCTL_DSP_SETTRIGGER/20230921-215828
base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next
patch link: https://lore.kernel.org/r/20230921064258.3582115-1-make_ruc2021%40163.com
patch subject: [PATCH v2] ALSA: pcm: oss: Fix race at SNDCTL_DSP_SETTRIGGER
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230921/202309212328.2UOE4Raf-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230921/202309212328.2UOE4Raf-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309212328.2UOE4Raf-lkp@intel.com/
All warnings (new ones prefixed by >>):
sound/core/oss/pcm_oss.c: In function 'snd_pcm_oss_set_trigger':
>> sound/core/oss/pcm_oss.c:2092:17: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
2092 | if (err < 0)
| ^~
sound/core/oss/pcm_oss.c:2094:25: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
2094 | return err;
| ^~~~~~
sound/core/oss/pcm_oss.c:2126:17: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
2126 | if (err < 0)
| ^~
sound/core/oss/pcm_oss.c:2128:25: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
2128 | return err;
| ^~~~~~
vim +/if +2092 sound/core/oss/pcm_oss.c
2072
2073 static int snd_pcm_oss_set_trigger(struct snd_pcm_oss_file *pcm_oss_file, int trigger)
2074 {
2075 struct snd_pcm_runtime *runtime;
2076 struct snd_pcm_substream *psubstream = NULL, *csubstream = NULL;
2077 int err, cmd;
2078
2079 #ifdef OSS_DEBUG
2080 pr_debug("pcm_oss: trigger = 0x%x\n", trigger);
2081 #endif
2082
2083 psubstream = pcm_oss_file->streams[SNDRV_PCM_STREAM_PLAYBACK];
2084 csubstream = pcm_oss_file->streams[SNDRV_PCM_STREAM_CAPTURE];
2085
2086 if (psubstream) {
2087 runtime = psubstream->runtime;
2088 cmd = 0;
2089 if (mutex_lock_interruptible(&runtime->oss.params_lock))
2090 return -ERESTARTSYS;
2091 err = snd_pcm_oss_make_ready_locked(psubstream);
> 2092 if (err < 0)
2093 mutex_unlock(&runtime->oss.params_lock);
2094 return err;
2095 if (trigger & PCM_ENABLE_OUTPUT) {
2096 if (runtime->oss.trigger)
2097 goto _skip1;
2098 if (atomic_read(&psubstream->mmap_count))
2099 snd_pcm_oss_simulate_fill(psubstream,
2100 get_hw_ptr_period(runtime));
2101 runtime->oss.trigger = 1;
2102 runtime->start_threshold = 1;
2103 cmd = SNDRV_PCM_IOCTL_START;
2104 } else {
2105 if (!runtime->oss.trigger)
2106 goto _skip1;
2107 runtime->oss.trigger = 0;
2108 runtime->start_threshold = runtime->boundary;
2109 cmd = SNDRV_PCM_IOCTL_DROP;
2110 runtime->oss.prepare = 1;
2111 }
2112 _skip1:
2113 mutex_unlock(&runtime->oss.params_lock);
2114 if (cmd) {
2115 err = snd_pcm_kernel_ioctl(psubstream, cmd, NULL);
2116 if (err < 0)
2117 return err;
2118 }
2119 }
2120 if (csubstream) {
2121 runtime = csubstream->runtime;
2122 cmd = 0;
2123 if (mutex_lock_interruptible(&runtime->oss.params_lock))
2124 return -ERESTARTSYS;
2125 err = snd_pcm_oss_make_ready_locked(csubstream);
2126 if (err < 0)
2127 mutex_unlock(&runtime->oss.params_lock);
2128 return err;
2129 if (trigger & PCM_ENABLE_INPUT) {
2130 if (runtime->oss.trigger)
2131 goto _skip2;
2132 runtime->oss.trigger = 1;
2133 runtime->start_threshold = 1;
2134 cmd = SNDRV_PCM_IOCTL_START;
2135 } else {
2136 if (!runtime->oss.trigger)
2137 goto _skip2;
2138 runtime->oss.trigger = 0;
2139 runtime->start_threshold = runtime->boundary;
2140 cmd = SNDRV_PCM_IOCTL_DROP;
2141 runtime->oss.prepare = 1;
2142 }
2143 _skip2:
2144 mutex_unlock(&runtime->oss.params_lock);
2145 if (cmd) {
2146 err = snd_pcm_kernel_ioctl(csubstream, cmd, NULL);
2147 if (err < 0)
2148 return err;
2149 }
2150 }
2151 return 0;
2152 }
2153
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-21 15:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-21 6:42 [PATCH v2] ALSA: pcm: oss: Fix race at SNDCTL_DSP_SETTRIGGER Ma Ke
2023-09-21 13:29 ` Takashi Iwai
2023-09-21 15:20 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).