From: Joel Stanley <joel@jms.id.au>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>
Cc: Jeremy Kerr <jk@ozlabs.org>,
Christopher Bostic <cbostic@linux.vnet.ibm.com>,
Brad Bishop <bradleyb@fuzziesquirrel.com>,
Edward James <eajames@us.ibm.com>,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: [PATCH 08/10] fsi: master: Clarify master lifetimes & fix use-after-free in hub master
Date: Mon, 12 Feb 2018 15:45:47 +1030 [thread overview]
Message-ID: <20180212051549.8575-9-joel@jms.id.au> (raw)
In-Reply-To: <20180212051549.8575-1-joel@jms.id.au>
From: Jeremy Kerr <jk@ozlabs.org>
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: Jeremy Kerr <jk@ozlabs.org>
Tested-by: Christopher Bostic <cbostic@linux.vnet.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/fsi/fsi-master-hub.c | 21 ++++++++++++++++++---
drivers/fsi/fsi-master.h | 15 +++++++++++++++
2 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/drivers/fsi/fsi-master-hub.c b/drivers/fsi/fsi-master-hub.c
index 5e4cd3134bc0..5885fc4a1ef0 100644
--- a/drivers/fsi/fsi-master-hub.c
+++ b/drivers/fsi/fsi-master-hub.c
@@ -288,10 +288,19 @@ static int hub_master_probe(struct device *dev)
hub_master_init(hub);
rc = fsi_master_register(&hub->master);
- if (!rc)
- return 0;
+ 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;
}
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index 18bd4ad79356..ee0b46086026 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -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);
--
2.15.1
next prev parent reply other threads:[~2018-02-12 5:15 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-12 5:15 [PATCH 00/10] FSI enhancements for v4.17 Joel Stanley
2018-02-12 5:15 ` [PATCH 01/10] fsi: Add fsi_master_rescan() Joel Stanley
2018-02-12 5:15 ` [PATCH 02/10] fsi: master-gpio: Add locking during break and link enable Joel Stanley
2018-02-12 5:15 ` [PATCH 03/10] fsi: master-gpio: Add external mode Joel Stanley
[not found] ` <20180212051549.8575-1-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
2018-02-12 5:15 ` [PATCH 04/10] fsi: Fix one and two byte bus reads/writes Joel Stanley
2018-02-12 5:15 ` [PATCH 05/10] dt-bindings: fsi: Add specification for FSI busses Joel Stanley
2018-02-12 5:15 ` [PATCH 06/10] fsi: Match fsi slaves and engines to available dt nodes Joel Stanley
2018-02-12 5:15 ` [PATCH 07/10] fsi: core: Reduce console output during normal scan Joel Stanley
2018-02-12 5:15 ` Joel Stanley [this message]
2018-02-12 5:15 ` [PATCH 09/10] dt-bindings: fsi: Add optional property no-scan-on-init Joel Stanley
[not found] ` <20180212051549.8575-10-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
2018-02-12 14:36 ` Rob Herring
2018-02-13 2:49 ` Joel Stanley
2018-02-12 5:15 ` [PATCH 10/10] fsi: core: Add check for master " Joel Stanley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180212051549.8575-9-joel@jms.id.au \
--to=joel@jms.id.au \
--cc=bradleyb@fuzziesquirrel.com \
--cc=cbostic@linux.vnet.ibm.com \
--cc=devicetree@vger.kernel.org \
--cc=eajames@us.ibm.com \
--cc=gregkh@linuxfoundation.org \
--cc=jk@ozlabs.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).