Re: [PATCH RFC 0/6] ALSA: Fix UAF with delayed kobj release

[Takashi Iwai <tiwai@suse.de>]

On Wed, 09 Aug 2023 10:10:04 +0200,
Takashi Iwai wrote:
> 
> On Tue, 08 Aug 2023 21:26:55 +0200,
> Curtis Malainey wrote:
> > 
> > On Mon, Aug 7, 2023 at 3:34 PM Curtis Malainey <cujomalainey@google.com> wrote:
> > >
> > > > It's just a RFC and only lightly tested.
> > >
> > > Thanks for the series
> > >
> > > I will be hammering this in my test setup for next several hours
> > 
> > Testing has yielded 0 bugs overnight.
> > 
> > After discussion it seems like this might be more of a workaround for
> > the APIs than properly using them. Adding Stephen for more input but
> > having two kobj in the same allocation is technically not correct as
> > you essentially refcounting the same thing twice. Also having an empty
> > release function essentially nullifies the purpose of the refcounts.
> > We should probably consider something that uses the API as intended
> > rather than trying to fight their function.
> 
> Moving each PCM device and control device to own object and properly
> release in the own device release could be another way to go.
> 
> OTOH, I'm still wondering whether how to assure synchronization if all
> device releases are done asynchronously.  If there are some
> dependencies between the resources (e.g. taking the parent's lock) at
> release, and how can it be guaranteed to work?  Or, the release calls
> must not touch anything outside its own?  If so, we'll still need to
> two places to finish the stuff: quiesce and release.

And now looking back at kobj code and device code, they do refcount
parent objects.  Maybe the problem is in our side -- the all devices
are created with the original real device as the parent, including the
card_dev, while there are some dependencies among children.  So, if we
build up a proper tree, pci_dev -> card_dev -> ctl_dev, pcm_dev, etc,
one of the problems could be solved.  It's more or less similar as
what I suggested initially (referring card_dev at pcm), while changing
the parent would make it implicitly.



Takashi