Takashi Iwai a écrit :
> At Mon, 15 Oct 2012 19:41:40 +0200,
> Matthieu CASTET wrote:
>> Hi Takashi,
>>
>> But I believe I found other races in the alsa char device handling. With the
>> attached patch, if you disconnect the usb audio device between "msleep o" and
>> "msleep o+", you will free the card resource (snd_card_do_free) and then use it [1].
>>
>> I did in in snd_ctl_open, but the same thing could be done in snd_pcm_open, ...
>
> OK, we'd need a generic open/close protection.
> For PCM, there is already a fix in my last patchset, so it should
> work, but for other devices, the paths are still uncovered.
>
I don't think it will work for pcm : the begin of snd_pcm_open is not protected
by any lock even with your patch
In snd_open we release sound_mutex before calling file->f_op->open.
The snd_lookup_minor_data and snd_card_file_add should be protected by a lock.
Attached a patch (pcm.crash) that help to trigger [1].
One idea could be to do the snd_card_file_add in snd_lookup_minor_data : it will
be protected by sound_mutex.
That what rfc attachement does (with debug printk and delay) : it add a
snd_lookup_minor_data2 that do the snd_card_file_add.
Matthieu
[1]
# cat /dev/pcmC0D0c
[ 572.187866] msleep o cf1e1200
[ 573.534454] usb 1-2: USB disconnect, device number 2
[ 573.543670] usb 1-2.1: USB disconnect, device number 3
[ 573.561645] usb 1-2.1.2: USB disconnect, device number 4
[ 577.337005] msleep o+
[ 577.339477] Unable to handle kernel paging request at virtual address 6b6b6d87
[ 577.347106] pgd = cccf4000
[ 577.349975] [6b6b6d87] *pgd=00000000
[ 577.353759] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[ 577.359374] Modules linked in:
[ 577.362640] CPU: 0 Not tainted (3.7.0-rc1-00004-gd6355f5-dirty #27)
[ 577.369628] PC is at __lock_acquire.clone.19+0x150/0xd40
[ 577.375274] LR is at lock_acquire+0x5c/0x70
[ 577.379699] pc : [<c0088b10>] lr : [<c0089cd0>] psr: 80000093
[ 577.379699] sp : cca01c28 ip : 00000001 fp : cca01c84
[ 577.391784] r10: c05d5814 r9 : 6b6b6d83 r8 : 00000080
[ 577.397308] r7 : cf15f480 r6 : 00000000 r5 : c05b67bc r4 : cca00000
[ 577.404205] r3 : 00000000 r2 : 00000000 r1 : 00000000 r0 : 6b6b6d83
[ 577.411102] Flags: Nzcv IRQs off FIQs on Mode SVC_32 ISA ARM Segment user
[ 577.418731] Control: 10c5387d Table: 8ccf4019 DAC: 00000015
[ 577.424804] Process cat (pid: 642, stack limit = 0xcca00240)
[ 577.430786] Stack: (0xcca01c28 to 0xcca02000)
[ 577.435394] 1c20: 00000001 20000093 000000c4 00000000
c05ae4a8 00000001
[ 577.444030] 1c40: 00000000 c05d4b40 00000009 c05ae478 60000013 00000004
cca01ccc 00000000
[ 577.452667] 1c60: cca00000 60000013 cf15f480 00000001 00000024 cf7bcd40
cca01cb4 cca01c88
[ 577.461334] 1c80: c0089cd0 c00889cc 00000000 00000000 c02eacec 00000000
6b6b6d73 c02eacec
[ 577.469970] 1ca0: ccd12f00 6b6b6d73 cca01cdc cca01cb8 c03e5234 c0089c80
00000001 00000000
[ 577.478607] 1cc0: c02eacec cf0980a8 6b6b6b6b cf0980a8 cca01cfc cca01ce0
c02eacec c03e51f8
[ 577.487243] 1ce0: cf0980a8 ffffffff c05b65d4 cf1e1200 cca01d4c cca01d00
c02fbb84 c02eaca8
[ 577.495880] 1d00: 22222222 cf1e1200 00000018 cd9d0120 07400018 c09d9af8
cca01d34 cca01d28
[ 577.504547] 1d20: c03e33fc cf0980a8 cd9d0120 cd9d0120 07400018 c09d9af8
00000024 cf7bcd40
[ 577.513183] 1d40: cca01d64 cca01d50 c02fbd90 c02fbb20 c0411378 cf0980a8
cca01d8c cca01d68
[ 577.521820] 1d60: c02ea810 c02fbd58 cd9d0120 cf0980a8 cf1fb1c0 00000000
c00f505c 00000000
[ 577.530456] 1d80: cca01db4 cca01d90 c00f50f4 c02ea774 c00f25cc 00000018
cf0980a8 cd9d0120
[ 577.539093] 1da0: cf0980b0 00020001 cca01ddc cca01db8 c00ef404 c00f5068
cca01ea0 cca01f60
[ 577.547729] 1dc0: 00000000 00020000 cca01e98 00000000 cca01df4 cca01de0
c00ef4c0 c00ef240
[ 577.556365] 1de0: cca01ea0 cca01ee0 cca01e6c cca01df8 c00fe348 c00ef4a4
cca01e6c cca01e08
[ 577.565002] 1e00: c00fb704 c00fb4a8 cca01e3c cca01e18 c004b2c8 c007d99c
00000000 cca01ee8
[ 577.573638] 1e20: 00000000 00000000 ceb2f178 cf0980a8 cf016e58 ceb2f178
cd9d0120 cf2fa830
[ 577.582275] 1e40: 386d45b8 cca01ee0 ceb42bf8 cca01e98 cca01f60 00000000
cca01ea0 cca00000
[ 577.590911] 1e60: cca01ed4 cca01e70 c00fec00 c00fdd70 cca01ea0 00000000
00000004 00000000
[ 577.599548] 1e80: 000000e5 00000000 cf0980a8 cca01e90 cf016e58 ceb42bf8
cf016e58 ceb40d38
[ 577.608215] 1ea0: 00000000 00000000 cca01ee4 cca01f60 00000001 ffffff9c
cf30f000 ffffff9c
[ 577.616851] 1ec0: cca00000 00000000 cca01f54 cca01ed8 c00ff0f8 c00fe958
00000041 cf0235e0
[ 577.625488] 1ee0: cf016e58 ceb40d38 d24edc20 00000008 ce487d44 cf7b3d90
cf016318 ce8021f8
[ 577.634124] 1f00: cd9d0120 00000101 00000004 00000000 00000000 ce487d40
00020000 cf30f000
[ 577.642761] 1f20: 00020000 be9ecf92 00000001 ffffff9c cca00000 00000000
cf30f000 00020000
[ 577.651397] 1f40: 00000003 00000001 cca01f94 cca01f58 c00f044c c00ff0d0
c0060598 c03e44d0
[ 577.660034] 1f60: 00020000 cca00000 00000024 00000100 00000000 00000000
be9ecf92 00000005
[ 577.668701] 1f80: c0014808 00000000 cca01fa4 cca01f98 c00f0504 c00f036c
00000000 cca01fa8
[ 577.677337] 1fa0: c0014660 c00f04e8 00000000 00000000 be9ecf92 00020000
00000000 000030ec
[ 577.685974] 1fc0: 00000000 00000000 be9ecf92 00000005 00000008 00000000
b6f5f000 00000000
[ 577.694610] 1fe0: b6e163b0 be9eccd0 0000d898 b6e16400 60000010 be9ecf92
00000000 00000000
[ 577.703247] Backtrace:
[ 577.705841] [<c00889c0>] (__lock_acquire.clone.19+0x0/0xd40) from
[<c0089cd0>] (lock_acquire+0x5c/0x70)
[ 577.715789] [<c0089c74>] (lock_acquire+0x0/0x70) from [<c03e5234>]
(_raw_spin_lock+0x48/0x58)
[ 577.724792] r7:6b6b6d73 r6:ccd12f00 r5:c02eacec r4:6b6b6d73
[ 577.730834] [<c03e51ec>] (_raw_spin_lock+0x0/0x58) from [<c02eacec>]
(snd_card_file_add+0x50/0xac)
[ 577.740295] r5:cf0980a8 r4:6b6b6b6b
[ 577.744110] [<c02eac9c>] (snd_card_file_add+0x0/0xac) from [<c02fbb84>]
(snd_pcm_open+0x70/0x238)
[ 577.753479] r7:cf1e1200 r6:c05b65d4 r5:ffffffff r4:cf0980a8
[ 577.759521] [<c02fbb14>] (snd_pcm_open+0x0/0x238) from [<c02fbd90>]
(snd_pcm_capture_open+0x44/0x48)
[ 577.769165] [<c02fbd4c>] (snd_pcm_capture_open+0x0/0x48) from [<c02ea810>]
(snd_open+0xa8/0x1a0)
[ 577.778442] r5:cf0980a8 r4:c0411378
[ 577.782257] [<c02ea768>] (snd_open+0x0/0x1a0) from [<c00f50f4>]
(chrdev_open+0x98/0x160)
[ 577.790832] [<c00f505c>] (chrdev_open+0x0/0x160) from [<c00ef404>]
(do_dentry_open.clone.16+0x1d0/0x264)
[ 577.800842] r7:00020001 r6:cf0980b0 r5:cd9d0120 r4:cf0980a8
[ 577.806884] [<c00ef234>] (do_dentry_open.clone.16+0x0/0x264) from
[<c00ef4c0>] (finish_open+0x28/0x40)
[ 577.816711] [<c00ef498>] (finish_open+0x0/0x40) from [<c00fe348>]
(do_last.clone.47+0x5e4/0xbe8)
[ 577.825988] r4:cca01ee0 r3:cca01ea0
[ 577.829803] [<c00fdd64>] (do_last.clone.47+0x0/0xbe8) from [<c00fec00>]
(path_openat.clone.48+0x2b4/0x488)
[ 577.839996] [<c00fe94c>] (path_openat.clone.48+0x0/0x488) from [<c00ff0f8>]
(do_filp_open+0x34/0x88)
[ 577.849670] [<c00ff0c4>] (do_filp_open+0x0/0x88) from [<c00f044c>]
(do_sys_open+0xec/0x17c)
[ 577.858489] r7:00000001 r6:00000003 r5:00020000 r4:cf30f000
[ 577.864532] [<c00f0360>] (do_sys_open+0x0/0x17c) from [<c00f0504>]
(sys_open+0x28/0x2c)
[ 577.872985] [<c00f04dc>] (sys_open+0x0/0x2c) from [<c0014660>]
(ret_fast_syscall+0x0/0x30)
[ 577.881744] Code: ebfeebea e1a00004 ea000038 e0890106 (e5908004)
[ 577.888183] ---[ end trace 5c43a835149de569 ]---
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 8753c89..0cbe471 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2130,11 +2130,16 @@ static int snd_pcm_capture_open(struct inode *inode, struct file *file)
return snd_pcm_open(file, pcm, SNDRV_PCM_STREAM_CAPTURE);
}
+#include <linux/delay.h>
static int snd_pcm_open(struct file *file, struct snd_pcm *pcm, int stream)
{
int err;
wait_queue_t wait;
+ printk("msleep o %p\n", pcm);
+ mdelay(5000);
+ printk("msleep o+\n");
+
if (pcm == NULL) {
err = -ENODEV;
goto __error1;
diff --git a/include/sound/core.h b/include/sound/core.h
index bc05668..9869087 100644
--- a/include/sound/core.h
+++ b/include/sound/core.h
@@ -185,6 +185,7 @@ static inline int snd_power_wait(struct snd_card *card, unsigned int state) { re
struct snd_minor {
int type; /* SNDRV_DEVICE_TYPE_XXX */
int card; /* card number */
+ struct snd_card *cardp;
int device; /* device number */
const struct file_operations *f_ops; /* file operations */
void *private_data; /* private data for f_ops->open */
@@ -241,6 +242,7 @@ static inline int snd_register_device(int type, struct snd_card *card, int dev,
int snd_unregister_device(int type, struct snd_card *card, int dev);
void *snd_lookup_minor_data(unsigned int minor, int type);
+void *snd_lookup_minor_data2(unsigned int minor, int type, struct file *file);
int snd_add_device_sysfs_file(int type, struct snd_card *card, int dev,
struct device_attribute *attr);
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 8753c89..c2445c0 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2114,8 +2114,8 @@ static int snd_pcm_playback_open(struct inode *inode, struct file *file)
int err = nonseekable_open(inode, file);
if (err < 0)
return err;
- pcm = snd_lookup_minor_data(iminor(inode),
- SNDRV_DEVICE_TYPE_PCM_PLAYBACK);
+ pcm = snd_lookup_minor_data2(iminor(inode),
+ SNDRV_DEVICE_TYPE_PCM_PLAYBACK, file);
return snd_pcm_open(file, pcm, SNDRV_PCM_STREAM_PLAYBACK);
}
@@ -2125,23 +2125,24 @@ static int snd_pcm_capture_open(struct inode *inode, struct file *file)
int err = nonseekable_open(inode, file);
if (err < 0)
return err;
- pcm = snd_lookup_minor_data(iminor(inode),
- SNDRV_DEVICE_TYPE_PCM_CAPTURE);
+ pcm = snd_lookup_minor_data2(iminor(inode),
+ SNDRV_DEVICE_TYPE_PCM_CAPTURE, file);
return snd_pcm_open(file, pcm, SNDRV_PCM_STREAM_CAPTURE);
}
+#include <linux/delay.h>
static int snd_pcm_open(struct file *file, struct snd_pcm *pcm, int stream)
{
int err;
wait_queue_t wait;
+ printk("msleep o2 %p\n", pcm);
+ mdelay(5000);
+ printk("msleep o2+\n");
if (pcm == NULL) {
err = -ENODEV;
goto __error1;
}
- err = snd_card_file_add(pcm->card, file);
- if (err < 0)
- goto __error1;
if (!try_module_get(pcm->card->module)) {
err = -EFAULT;
goto __error2;
diff --git a/sound/core/sound.c b/sound/core/sound.c
index 6439760..5ea8010 100644
--- a/sound/core/sound.c
+++ b/sound/core/sound.c
@@ -118,6 +118,35 @@ void *snd_lookup_minor_data(unsigned int minor, int type)
EXPORT_SYMBOL(snd_lookup_minor_data);
+#include <linux/delay.h>
+void *snd_lookup_minor_data2(unsigned int minor, int type, struct file *file)
+{
+ struct snd_minor *mreg;
+ void *private_data;
+
+ if (minor >= ARRAY_SIZE(snd_minors))
+ return NULL;
+ mutex_lock(&sound_mutex);
+ mreg = snd_minors[minor];
+ if (mreg && mreg->type == type)
+ private_data = mreg->private_data;
+ else
+ private_data = NULL;
+ printk("msleep o %p\n", NULL);
+ mdelay(5000);
+ printk("msleep o+\n");
+
+
+ if (private_data) {
+ if (snd_card_file_add(mreg->cardp, file) < 0)
+ private_data = NULL;
+ }
+ mutex_unlock(&sound_mutex);
+ return private_data;
+}
+
+EXPORT_SYMBOL(snd_lookup_minor_data2);
+
#ifdef CONFIG_MODULES
static struct snd_minor *autoload_device(unsigned int minor)
{
@@ -272,6 +301,7 @@ int snd_register_device_for_dev(int type, struct snd_card *card, int dev,
return -ENOMEM;
preg->type = type;
preg->card = card ? card->number : -1;
+ preg->cardp = card;
preg->device = dev;
preg->f_ops = f_ops;
preg->private_data = private_data;
