Commit e0c24bdd authored by Jeremy Kerr's avatar Jeremy Kerr Committed by Greg Kroah-Hartman
Browse files

fsi: master: Clarify master lifetimes & fix use-after-free in hub master 



Once we call fsi_master_unregister, the core will put_device,
potentially freeing the hub master. This change adds a comment
explaining the lifetime of an allocated fsi_master.

We then add a reference from the driver to the hub master, so it stays
around until we've finished ->remove().

Signed-off-by: default avatarJeremy Kerr <jk@ozlabs.org>
Tested-by: default avatarChristopher Bostic <cbostic@linux.vnet.ibm.com>
Signed-off-by: default avatarJoel Stanley <joel@jms.id.au>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 638bd9ac

drivers/fsi/fsi-master-hub.c

+18 −3
Original line number Diff line number Diff line
@@ -288,10 +288,19 @@ static int hub_master_probe(struct device *dev) 
	hub_master_init(hub);



	rc = fsi_master_register(&hub->master);

	if (!rc)

	if (rc)

		goto err_release;



	/* At this point, fsi_master_register performs the device_initialize(),

	 * and holds the sole reference on master.dev. This means the device

	 * will be freed (via ->release) during any subsequent call to

	 * fsi_master_unregister.  We add our own reference to it here, so we

	 * can perform cleanup (in _remove()) without it being freed before

	 * we're ready.

	 */

	get_device(&hub->master.dev);

	return 0;



	kfree(hub);

err_release:

	fsi_slave_release_range(fsi_dev->slave, FSI_HUB_LINK_OFFSET,

			FSI_HUB_LINK_SIZE * links);
@@ -306,6 +315,12 @@ static int hub_master_remove(struct device *dev) 
	fsi_slave_release_range(hub->upstream->slave, hub->addr, hub->size);

	of_node_put(hub->master.dev.of_node);



	/*

	 * master.dev will likely be ->release()ed after this, which free()s

	 * the hub

	 */

	put_device(&hub->master.dev);



	return 0;

}

drivers/fsi/fsi-master.h

+15 −0
Original line number Diff line number Diff line
@@ -37,6 +37,21 @@ struct fsi_master { 


#define dev_to_fsi_master(d) container_of(d, struct fsi_master, dev)



/**

 * fsi_master registration & lifetime: the fsi_master_register() and

 * fsi_master_unregister() functions will take ownership of the master, and

 * ->dev in particular. The registration path performs a get_device(), which

 * takes the first reference on the device. Similarly, the unregistration path

 * performs a put_device(), which may well drop the last reference.

 *

 * This means that master implementations *may* need to hold their own

 * reference (via get_device()) on master->dev. In particular, if the device's

 * ->release callback frees the fsi_master, then fsi_master_unregister will

 * invoke this free if no other reference is held.

 *

 * The same applies for the error path of fsi_master_register; if the call

 * fails, dev->release will have been invoked.

 */

extern int fsi_master_register(struct fsi_master *master);

extern void fsi_master_unregister(struct fsi_master *master);

CRA Git | Maintained and supported by SUSTech CRA and CCSE