At Sat, 21 Feb 2009 10:55:30 -0600, Brad Bosch wrote:
I discovered and fixed an ALSA oops in a very old kernel (2.6.5) used in an embedded application and looking at the latest kernels, it's not clear to me that this was ever completely fixed in newer kernels. I'm sending this email so someone who understands the interactions between the USB and ALSA subsystems better than I do can determine if a similar fix might be required in the newer kernels.
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.
In the old kernel, the oops often happens when a USB device is unplugged and then a device with the same ID (or the same device) is immediately plugged back in before the old device is completely cleaned up. The included patch resolves the problem in that kernel.
The problem was that two cards were creating symbolic links in /proc/asound with the same name (id) but pointing to differen't cards.
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.
The proc subsystem apparently allowed this. Later, when one of the entries is removed (using it's name), the wrong entry may be removed, freeing the wrong struct proc_dir_entry and leaving a pointer to the freed memory in the other card's proc_root_link member. Later when that USB device is unplugged and snd_info_card_disconnect() is called, snd_remove_proc_entry() will reference the name member of struct proc_dir_entry which by now may be changed, or even null. It turns out to be trivial to avoid this problem by simply not storing the pointer to the proc_dir_entry as we don't need it anyway to determine the name of the link to remove. Instead, for the case of the symbolic link, we call remove_proc_entry directly using the card id for the link name (just as we did when it was created).
I don't know if two symbolic links are ever created in /proc/asound with the same name in the newer kernels or newer ALSA code and in fact, I haven't been able to reproduce the problem there. However, since the patch reduces both the data and code size, and eliminates a potential crash, you may want to apply it anyway.
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 :)
thanks,
Takashi
--Brad
Index: sound/core/info.c
RCS file: /cvs/kernels/kernel_2_6_5/sound/core/info.c,v retrieving revision 1.1.4.1.10.1 diff -u -r1.1.4.1.10.1 info.c --- sound/core/info.c 22 Feb 2006 14:06:25 -0000 1.1.4.1.10.1 +++ sound/core/info.c 20 Feb 2009 02:05:01 -0000 @@ -618,7 +618,6 @@ p = proc_symlink(card->id, snd_proc_root, card->proc_root->name); if (p == NULL) return -ENOMEM;
- card->proc_root_link = p; return 0;
}
@@ -629,10 +628,7 @@ int snd_info_card_free(snd_card_t * card) { snd_assert(card != NULL, return -ENXIO);
- 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_unregister(card->proc_root); card->proc_root = NULL;
Index: include/sound/core.h
RCS file: /cvs/kernels/kernel_2_6_5/include/sound/core.h,v retrieving revision 1.1.4.1 diff -u -r1.1.4.1 core.h --- include/sound/core.h 15 Apr 2005 21:45:13 -0000 1.1.4.1 +++ include/sound/core.h 20 Feb 2009 02:05:02 -0000 @@ -152,7 +152,6 @@
snd_info_entry_t *proc_root; /* root for soundcard specific files */ snd_info_entry_t *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 state */
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel