Skip to content

Commit 96e503f

Browse files
committed
ALSA: hda/i915 - fix list corruption with concurrent probes
Current hdac_i915 uses a static completion instance to wait for i915 driver to complete the component bind. This design is not safe if multiple HDA controllers are active and communicating with different i915 instances, and can lead to list corruption and failed audio driver probe. Fix the design by moving completion mechanism to common acomp code and remove the related code from hdac_i915. Fixes: 7b882fe ("ALSA: hda - handle multiple i915 device instances") Co-developed-by: Kai Vehmanen <[email protected]> Signed-off-by: Kai Vehmanen <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Takashi Iwai <[email protected]>
1 parent 7dcd561 commit 96e503f

File tree

3 files changed

+10
-20
lines changed

3 files changed

+10
-20
lines changed

include/drm/drm_audio_component.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,10 @@ struct drm_audio_component {
117117
* @audio_ops: Ops implemented by hda driver, called by DRM driver
118118
*/
119119
const struct drm_audio_component_audio_ops *audio_ops;
120+
/**
121+
* @master_bind_complete: completion held during component master binding
122+
*/
123+
struct completion master_bind_complete;
120124
};
121125

122126
#endif /* _DRM_AUDIO_COMPONENT_H_ */

sound/hda/hdac_component.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,12 +210,14 @@ static int hdac_component_master_bind(struct device *dev)
210210
goto module_put;
211211
}
212212

213+
complete_all(&acomp->master_bind_complete);
213214
return 0;
214215

215216
module_put:
216217
module_put(acomp->ops->owner);
217218
out_unbind:
218219
component_unbind_all(dev, acomp);
220+
complete_all(&acomp->master_bind_complete);
219221

220222
return ret;
221223
}
@@ -296,6 +298,7 @@ int snd_hdac_acomp_init(struct hdac_bus *bus,
296298
if (!acomp)
297299
return -ENOMEM;
298300
acomp->audio_ops = aops;
301+
init_completion(&acomp->master_bind_complete);
299302
bus->audio_component = acomp;
300303
devres_add(dev, acomp);
301304

sound/hda/hdac_i915.c

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111
#include <sound/hda_i915.h>
1212
#include <sound/hda_register.h>
1313

14-
static struct completion bind_complete;
15-
1614
#define IS_HSW_CONTROLLER(pci) (((pci)->device == 0x0a0c) || \
1715
((pci)->device == 0x0c0c) || \
1816
((pci)->device == 0x0d0c) || \
@@ -130,19 +128,6 @@ static bool i915_gfx_present(void)
130128
return pci_dev_present(ids);
131129
}
132130

133-
static int i915_master_bind(struct device *dev,
134-
struct drm_audio_component *acomp)
135-
{
136-
complete_all(&bind_complete);
137-
/* clear audio_ops here as it was needed only for completion call */
138-
acomp->audio_ops = NULL;
139-
return 0;
140-
}
141-
142-
static const struct drm_audio_component_audio_ops i915_init_ops = {
143-
.master_bind = i915_master_bind
144-
};
145-
146131
/**
147132
* snd_hdac_i915_init - Initialize i915 audio component
148133
* @bus: HDA core bus
@@ -163,9 +148,7 @@ int snd_hdac_i915_init(struct hdac_bus *bus)
163148
if (!i915_gfx_present())
164149
return -ENODEV;
165150

166-
init_completion(&bind_complete);
167-
168-
err = snd_hdac_acomp_init(bus, &i915_init_ops,
151+
err = snd_hdac_acomp_init(bus, NULL,
169152
i915_component_master_match,
170153
sizeof(struct i915_audio_component) - sizeof(*acomp));
171154
if (err < 0)
@@ -177,8 +160,8 @@ int snd_hdac_i915_init(struct hdac_bus *bus)
177160
if (!IS_ENABLED(CONFIG_MODULES) ||
178161
!request_module("i915")) {
179162
/* 60s timeout */
180-
wait_for_completion_timeout(&bind_complete,
181-
msecs_to_jiffies(60 * 1000));
163+
wait_for_completion_timeout(&acomp->master_bind_complete,
164+
msecs_to_jiffies(60 * 1000));
182165
}
183166
}
184167
if (!acomp->ops) {

0 commit comments

Comments
 (0)