Since you're going to have to resend the patch anyway could you modify this commit message some more?
On Wed, Aug 02, 2023 at 10:43:49AM -0700, cujomalainey@chromium.org wrote:
From: Curtis Malainey cujomalainey@chromium.org
The current implementation of how devices are released is valid for production use cases (root control of memory is handled by card_dev, all other devices are no-ops).
I don't understand what "root control of memory is handled by card_dev, all other devices are no-ops" means. At first I thought this was refering to code that is out of tree but now I think we are talking about a CONFIG_DEBUG option. Could you spell out which option we are talking about?
This model does not work though in a kernel hacking environment where KASAN and delayed release on kobj is enabled.
I don't think KASAN has anything to do with the bug, right? KASAN just finds the bug, it doesn't cause it. The bug is always there regardless. The "delayed release" is CONFIG_DEBUG_KOBJECT_RELEASE. Could you please mention that specifically. Say something like:
"KASAN detected a use after free when CONFIG_DEBUG_KOBJECT_RELEASE is enabled, which, hopefully, no one does on a production system."
I feel like a KASAN stack trace might help clarify where the use after free happens.
If the card_dev device is released before any of the child objects a use-after-free bug is caught by KASAN as the delayed release still has a reference to the devices that were freed by the card_dev release.
Ah... I think I understand.
"The CONFIG_DEBUG_KOBJECT_RELEASE introduces an element of randomness to the release process so we could free the card_dev before the child objects resulting in a use after free. But if we don't enable that the releases happen in a nice fixed order."
Also both snd_card and snd_pcm both own two devices internally, so even if they released independently, the shared struct would result in another use after free.
Does this second use after free happen regardless of CONFIG_DEBUG_KOBJECT_RELEASE?
Solution is to move the child devices into their own memory so they can be handled independently and released on their own schedule.
Signed-off-by: Curtis Malainey cujomalainey@chromium.org Cc: Doug Anderson dianders@chromium.org
Also I know it's complicated here, but could you try identify a Fixes tag where this bug is introduced or first starts affecting the things? This looks like a pretty core bug so it's possible it predates git. I'm not sure what to do in that case. I normally just mention it under the --- cut off line.
regards, dan carpenter