Re: [PATCH v5 4/5] Initialize TPM and get durations and timeouts

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

On Fri, Feb 12, 2016 at 05:23:16PM -0500, Stefan Berger wrote:

>    > I don't see how that can happen. Looking at the tpm cdev, it is

>    Well, following my tests, it does happen.

How are you testing for use-after-free bugs? Did you test the kapi
interface? Did you get it in *exactly* the right configuration to
cause this?

>    > So, we have a situation where tpm_unregister can return, release the
>    > kref on the vtpm and still have outstanding users, which will result
>    > in a use after-free.

>    Maybe you should give it a try ... I don't see these problems and any
>    change seems like ill-fixing it. What will be accessed after
>    tpm_unregister? The only case we have tpm_unregister is here:

>    static void vtpm_delete_device_pair(structvtpm_dev *vtpm_dev)
>    {
>            tpm_chip_unregister(vtpm_dev->chip);
>            vtpm_fops_undo_open(vtpm_dev);
>            vtpm_delete_vtpm_dev(vtpm_dev);
>    }

>    followed by:

>    static inline voidvtpm_delete_vtpm_dev(struct vtpm_dev *vtpm_dev)
>    {
>            put_device(&vtpm_dev->chip->dev); /* frees chip */
>            device_unregister(&vtpm_dev->dev); /* does put_device */
>    }

>    I don't see a problem with that.

device_unregister will put_device(vtpmt_dev) which will kfree it since
no refs are left.

tpm_chip is still alive because the cdev/kapi are still holding a ref
on it.

The cdev/kapi can still call vtpm_tpm_op_send which will then deref
chip->priv which is the free'd vtpm_dev.

Any attempt to test for this scenario is very likely to hit the
STATE_OPENED_BIT test and exit without crashing. However that is just
a user-after free getting lucky. Testing is not a substitute for
proper analysis.

Show me an analysis of what in cdev/kapi is holding the kref on
vtpm_dev if you still think this can't happen.

As I said, the only kref the tpm core takes on vtpm_dev is dropped
by tpm_chip_unregister.

Jason

------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140