Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1341110
| From | Ben Hutchings <ben@decadent.org.uk> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | [PATCH 3.2 52/67] ALSA: dummy: Implement timer backend switching more safely |
| Date | 2016-02-23 23:10 +0100 |
| Message-ID | <r5tjB-3kK-37@gated-at.bofh.it> (permalink) |
| References | <r5t0e-2Ub-3@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
3.2.78-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: Takashi Iwai <tiwai@suse.de>
commit ddce57a6f0a2d8d1bfacfa77f06043bc760403c2 upstream.
Currently the selected timer backend is referred at any moment from
the running PCM callbacks. When the backend is switched, it's
possible to lead to inconsistency from the running backend. This was
pointed by syzkaller fuzzer, and the commit [7ee96216c31a: ALSA:
dummy: Disable switching timer backend via sysfs] disabled the dynamic
switching for avoiding the crash.
This patch improves the handling of timer backend switching. It keeps
the reference to the selected backend during the whole operation of an
opened stream so that it won't be changed by other streams.
Together with this change, the hrtimer parameter is reenabled as
writable now.
NOTE: this patch also turned out to fix the still remaining race.
Namely, ops was still replaced dynamically at dummy_pcm_open:
static int dummy_pcm_open(struct snd_pcm_substream *substream)
{
....
dummy->timer_ops = &dummy_systimer_ops;
if (hrtimer)
dummy->timer_ops = &dummy_hrtimer_ops;
Since dummy->timer_ops is common among all streams, and when the
replacement happens during accesses of other streams, it may lead to a
crash. This was actually triggered by syzkaller fuzzer and KASAN.
This patch rewrites the code not to use the ops shared by all streams
any longer, too.
BugLink: http://lkml.kernel.org/r/CACT4Y+aZ+xisrpuM6cOXbL21DuM0yVxPYXf4cD4Md9uw0C3dBQ@mail.gmail.com
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
sound/drivers/dummy.c | 37 +++++++++++++++++++------------------
1 file changed, 19 insertions(+), 18 deletions(-)
--- a/sound/drivers/dummy.c
+++ b/sound/drivers/dummy.c
@@ -87,7 +87,7 @@ MODULE_PARM_DESC(pcm_substreams, "PCM su
module_param(fake_buffer, bool, 0444);
MODULE_PARM_DESC(fake_buffer, "Fake buffer allocations.");
#ifdef CONFIG_HIGH_RES_TIMERS
-module_param(hrtimer, bool, 0444);
+module_param(hrtimer, bool, 0644);
MODULE_PARM_DESC(hrtimer, "Use hrtimer as the timer source.");
#endif
@@ -109,6 +109,9 @@ struct dummy_timer_ops {
snd_pcm_uframes_t (*pointer)(struct snd_pcm_substream *);
};
+#define get_dummy_ops(substream) \
+ (*(const struct dummy_timer_ops **)(substream)->runtime->private_data)
+
struct dummy_model {
const char *name;
int (*playback_constraints)(struct snd_pcm_runtime *runtime);
@@ -134,7 +137,6 @@ struct snd_dummy {
spinlock_t mixer_lock;
int mixer_volume[MIXER_ADDR_LAST+1][2];
int capture_source[MIXER_ADDR_LAST+1][2];
- const struct dummy_timer_ops *timer_ops;
};
/*
@@ -228,6 +230,8 @@ struct dummy_model *dummy_models[] = {
*/
struct dummy_systimer_pcm {
+ /* ops must be the first item */
+ const struct dummy_timer_ops *timer_ops;
spinlock_t lock;
struct timer_list timer;
unsigned long base_time;
@@ -365,6 +369,8 @@ static struct dummy_timer_ops dummy_syst
*/
struct dummy_hrtimer_pcm {
+ /* ops must be the first item */
+ const struct dummy_timer_ops *timer_ops;
ktime_t base_time;
ktime_t period_time;
atomic_t running;
@@ -491,31 +497,25 @@ static struct dummy_timer_ops dummy_hrti
static int dummy_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
{
- struct snd_dummy *dummy = snd_pcm_substream_chip(substream);
-
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
case SNDRV_PCM_TRIGGER_RESUME:
- return dummy->timer_ops->start(substream);
+ return get_dummy_ops(substream)->start(substream);
case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_SUSPEND:
- return dummy->timer_ops->stop(substream);
+ return get_dummy_ops(substream)->stop(substream);
}
return -EINVAL;
}
static int dummy_pcm_prepare(struct snd_pcm_substream *substream)
{
- struct snd_dummy *dummy = snd_pcm_substream_chip(substream);
-
- return dummy->timer_ops->prepare(substream);
+ return get_dummy_ops(substream)->prepare(substream);
}
static snd_pcm_uframes_t dummy_pcm_pointer(struct snd_pcm_substream *substream)
{
- struct snd_dummy *dummy = snd_pcm_substream_chip(substream);
-
- return dummy->timer_ops->pointer(substream);
+ return get_dummy_ops(substream)->pointer(substream);
}
static struct snd_pcm_hardware dummy_pcm_hardware = {
@@ -561,17 +561,19 @@ static int dummy_pcm_open(struct snd_pcm
struct snd_dummy *dummy = snd_pcm_substream_chip(substream);
struct dummy_model *model = dummy->model;
struct snd_pcm_runtime *runtime = substream->runtime;
+ const struct dummy_timer_ops *ops;
int err;
- dummy->timer_ops = &dummy_systimer_ops;
+ ops = &dummy_systimer_ops;
#ifdef CONFIG_HIGH_RES_TIMERS
if (hrtimer)
- dummy->timer_ops = &dummy_hrtimer_ops;
+ ops = &dummy_hrtimer_ops;
#endif
- err = dummy->timer_ops->create(substream);
+ err = ops->create(substream);
if (err < 0)
return err;
+ get_dummy_ops(substream) = ops;
runtime->hw = dummy->pcm_hw;
if (substream->pcm->device & 1) {
@@ -593,7 +595,7 @@ static int dummy_pcm_open(struct snd_pcm
err = model->capture_constraints(substream->runtime);
}
if (err < 0) {
- dummy->timer_ops->free(substream);
+ get_dummy_ops(substream)->free(substream);
return err;
}
return 0;
@@ -601,8 +603,7 @@ static int dummy_pcm_open(struct snd_pcm
static int dummy_pcm_close(struct snd_pcm_substream *substream)
{
- struct snd_dummy *dummy = snd_pcm_substream_chip(substream);
- dummy->timer_ops->free(substream);
+ get_dummy_ops(substream)->free(substream);
return 0;
}
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH 3.2 00/67] 3.2.78-rc1 review Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 10/67] sctp: allow setting SCTP_SACK_IMMEDIATELY by the application Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 14/67] USB: serial: option: Adding support for Telit LE922 Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 06/67] usb: cdc-acm: send zero packet for intel 7260 modem Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 02/67] hrtimer: Handle remaining time proper for TIME_LOW_RES Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 52/67] ALSA: dummy: Implement timer backend switching more safely Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 37/67] ALSA: rawmidi: Make snd_rawmidi_transmit() race-free Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 01/67] KVM: vmx: fix MPX detection Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 40/67] Revert "xhci: don't finish a TD if we get a short-transfer event mid TD" Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 38/67] ALSA: rawmidi: Fix race at copying & updating the position Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 41/67] usb: xhci: apply XHCI_PME_STUCK_QUIRK to Intel Broxton-M platforms Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 08/67] af_unix: fix struct pid memory leak Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 04/67] posix-timers: Handle relative timers with CONFIG_TIME_LOW_RES proper Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 45/67] [media] saa7134-alsa: Only frees registered sound cards Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 58/67] ahci: Intel DNV device IDs SATA Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 07/67] cdc-acm:exclude Samsung phone 04e8:685d Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 05/67] itimers: Handle relative timers with CONFIG_TIME_LOW_RES proper Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
[PATCH 3.2 46/67] scsi_dh_rdac: always retry MODE SELECT on command lock violation Ben Hutchings <ben@decadent.org.uk> - 2016-02-23 23:10 +0100
Re: [PATCH 3.2 00/67] 3.2.78-rc1 review Guenter Roeck <linux@roeck-us.net> - 2016-02-24 03:50 +0100
Re: [PATCH 3.2 00/67] 3.2.78-rc1 review Ben Hutchings <ben@decadent.org.uk> - 2016-02-24 15:50 +0100
csiph-web