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@xxxxxxxxxxxx wrote:
> From: Curtis Malainey <cujomalainey@xxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxx>
> Cc: Doug Anderson <dianders@xxxxxxxxxxxx>
> ---
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
