At Tue, 24 Feb 2009 10:50:35 -0600,
Brad Bosch wrote:
>
> Takashi Iwai writes:
> > Thanks for the patch! I'm very interested in whether the problem
> > happens with the very same scenario in the latest driver code.
> > There are still a few (but rare) bug reports, supposedly happening
> > with the same use-case. But, the disconnection code was largely
> > changed since 2.6.5, it's not clear at all whether it's the same
> > cause or not.
>
> If the oops stack trace for these bug reports looks something like
> this, I'd guess there may still be a race which allows this bug:
>
> Unable to handle kernel NULL pointer dereference at virtual address 0000004c
> printing eip:
> 02166027
> *pde = 00000000
> Oops: 0000 [#1]
> PREEMPT
> CPU: 0
> EIP: 0060:[<02166027>] Tainted: P
> EFLAGS: 00010286 (2.6.5-1.358pt)
> EIP is at remove_proc_entry+0x34/0xe5
> eax: 00000000 ebx: 3f87bc94 ecx: ffffffff edx: 0000004c
> esi: 154fca64 edi: 0000004c ebp: 41fa9ed8 esp: 41fa9ec8
> ds: 007b es: 007b ss: 0068
> Process events/0 (pid: 3, threadinfo=41fa9000 task=03f2eb50)
> Stack: 0000004c 3f87bc94 154fca64 00000001 41fa9ef8 431c9b09 0000004c 37ddd8c0
> 431cece5 3f87bc94 0000004c 154fca64 41fa9f0c 431ca648 37ddd8c0 3f87bc94
> 154fca64 41fa9f40 431c8cad 154fca64 00000000 03f2eb50 02111205 00100100
> Call Trace:
> [<431c9b09>] snd_remove_proc_entry+0x24/0x3c [snd]
> [<431ca648>] snd_info_card_free+0x7d/0xab [snd]
> [<431c8cad>] snd_card_free+0x17e/0x1f9 [snd]
Unfortunately, not.
It's somewhere in snd_mixer_oss_notify_handler() called in the delayed
snd_card_do_free().
> > Well, the sound core checks the duplicated id string, so I'm wondering
> > where the same id string comes from. Also, the recent kobject handler
> > checks the duplicated entry, and will spew the errors at
> > registration.
>
> Then I wouldn't expect to see the oops above in the latest kernels
> unless something else is deleting the symlink entries in /proc/asound
> But as you point out below, this patch shouldn't cause any trouble and
> I think it is an improvement anyway.
Yep.
> >
> > Yes, the fix sounds sane and likely applicable to the latest code.
> > At least it should be harmless...
> >
> > > This patch won't apply directly to newer kernels because of minor
> > > changes, but it's trivial to apply by hand. Let me know if you need
> > > me to re-base the patch.
> >
> > Yes, please. Then let's queue to the devel tree and see whether
> > any regression occurs actually :)
>
> OK. Here is the patch based on 2.6.28.7. I've applied, built, and
> tested it here and seen no ill effects.
Well, 2.6.29 has a feature to rename id string dynamically, and this
refers to the proc_root_link field. Thus your patch isn't applicable,
unfortunately.
We need a workaround for that, e.g. call snd_info_card_id_change()
before changing card->id...
thanks,
(BTW, it'd be appreciated if you make a diff with -p1 at the next
time.)
Takashi
> Thanks!
>
> --Brad
>
> Signed-off-by: Bradley Bosch <bradbosch@xxxxxxxxxxx>
>
> ChangeLog:
> Avoid caching a pointer to struct proc_dir_entry for the card symbolic
> links in /proc/asound and eliminate the proc_root_link member of
> struct snd_card. This avoids a potential oops should the link ever be
> freed by another card.
>
> --- sound/core/info.c~ 2009-02-20 16:41:27.000000000 -0600
> +++ sound/core/info.c 2009-02-23 18:09:44.000000000 -0600
> @@ -648,7 +648,6 @@ int snd_info_card_register(struct snd_ca
> p = proc_symlink(card->id, snd_proc_root, card->proc_root->name);
> if (p == NULL)
> return -ENOMEM;
> - card->proc_root_link = p;
> return 0;
> }
>
> @@ -661,10 +660,7 @@ void snd_info_card_disconnect(struct snd
> if (!card)
> return;
> mutex_lock(&info_mutex);
> - if (card->proc_root_link) {
> - snd_remove_proc_entry(snd_proc_root, card->proc_root_link);
> - card->proc_root_link = NULL;
> - }
> + remove_proc_entry(card->id, snd_proc_root);
> if (card->proc_root)
> snd_info_disconnect(card->proc_root);
> mutex_unlock(&info_mutex);
> --- include/sound/core.h~ 2009-02-20 16:41:27.000000000 -0600
> +++ include/sound/core.h 2009-02-23 18:10:34.000000000 -0600
> @@ -132,7 +132,6 @@ struct snd_card {
>
> struct snd_info_entry *proc_root; /* root for soundcard specific files */
> struct snd_info_entry *proc_id; /* the card id */
> - struct proc_dir_entry *proc_root_link; /* number link to real id */
>
> struct snd_monitor_file *files; /* all files associated to this card */
> struct snd_shutdown_f_ops *s_f_ops; /* file operations in the shutdown
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
