On Wed, Jul 08, 2015 at 03:33:25PM -0700, Greg Kroah-Hartman wrote:
> > The basic issue is that cdev_del doesn't seem to be synchronizing.
> >
> > The use after free race is then something like:
> >
> > struct tpm_chip {
> > struct device dev;
> > struct cdev cdev;
>
> Oops, right there's your problem. You can't have two reference counted
> objects trying to manage the memory of a single structure. No matter
> what you do, it's going to be a pain to deal with this, so don't :)
Sure, generally, yes, but that isn't done for no reason, it is to make
open straightforward:
static int tpm_open(struct inode *inode, struct file *file)
{
struct tpm_chip *chip =
container_of(inode->i_cdev, struct tpm_chip, cdev);
We need to recover the tpm_chip associated with the char device
node, in a way that is holding a kref on it, without racing with
cdev_del/etc
This scheme does mean that if we have a struct file we have a kref on
the cdev, and if we have cdev then we have a kref on the tpm_chip,
which is really easy to use properly.
> > Ie we need cdev to hold a ref on tpm_chip->dev until cdev_put is
> > called.
>
> No, separate them, make the cdev a pointer and all should be fine.
Okay, cdev_alloc takes care of the cdev lifetime.
Do you have a simple solution to replace container_of as well?
What would you think about something like:
cdev_alloc(&chip->dev.kref)
?
Jason
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
