All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
To: linux-media@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org,
	"Jacopo Mondi" <jacopo+renesas@jmondi.org>,
	"Kieran Bingham" <kieran.bingham+renesas@ideasonboard.com>,
	"Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
	"Ramesh Shanmugasundaram" <rashanmu@gmail.com>,
	"Sakari Ailus" <sakari.ailus@linux.intel.com>,
	"Steve Longerbeam" <slongerbeam@gmail.com>
Subject: [PATCH 5/5] media: i2c: max9286: Allocate v4l2_async_subdev dynamically
Date: Tue, 11 Aug 2020 23:59:39 +0300	[thread overview]
Message-ID: <20200811205939.19550-6-laurent.pinchart+renesas@ideasonboard.com> (raw)
In-Reply-To: <20200811205939.19550-1-laurent.pinchart+renesas@ideasonboard.com>

v4l2_async_notifier_add_subdev() requires the asd to be allocated
dynamically, but the max9286 driver embeds it in the max9286_source
structure. This causes memory corruption when the notifier is destroyed
at remove time with v4l2_async_notifier_cleanup().

Fix this issue by registering the asd with
v4l2_async_notifier_add_fwnode_subdev(), which allocates it dynamically
internally. A new max9286_asd structure is introduced, to store a
pointer to the corresonding max9286_source that needs to be accessed
from bound and unbind callbacks. There's no need to take an extra
explicit reference to the fwnode anymore as
v4l2_async_notifier_add_fwnode_subdev() does so internally.

While at it, use %u instead of %d to print the unsigned index in the
error message from the v4l2_async_notifier_add_fwnode_subdev() error
path.

Fixes: 66d8c9d2422d ("media: i2c: Add MAX9286 driver")
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/i2c/max9286.c | 38 +++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index 47f280518fdb..5d890dddb376 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -135,13 +135,19 @@
 #define MAX9286_SRC_PAD			4
 
 struct max9286_source {
-	struct v4l2_async_subdev asd;
 	struct v4l2_subdev *sd;
 	struct fwnode_handle *fwnode;
 };
 
-#define asd_to_max9286_source(_asd) \
-	container_of(_asd, struct max9286_source, asd)
+struct max9286_asd {
+	struct v4l2_async_subdev base;
+	struct max9286_source *source;
+};
+
+static inline struct max9286_asd *to_max9286_asd(struct v4l2_async_subdev *asd)
+{
+	return container_of(asd, struct max9286_asd, base);
+}
 
 struct max9286_priv {
 	struct i2c_client *client;
@@ -480,7 +486,7 @@ static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
 				struct v4l2_async_subdev *asd)
 {
 	struct max9286_priv *priv = sd_to_max9286(notifier->sd);
-	struct max9286_source *source = asd_to_max9286_source(asd);
+	struct max9286_source *source = to_max9286_asd(asd)->source;
 	unsigned int index = to_index(priv, source);
 	unsigned int src_pad;
 	int ret;
@@ -544,7 +550,7 @@ static void max9286_notify_unbind(struct v4l2_async_notifier *notifier,
 				  struct v4l2_async_subdev *asd)
 {
 	struct max9286_priv *priv = sd_to_max9286(notifier->sd);
-	struct max9286_source *source = asd_to_max9286_source(asd);
+	struct max9286_source *source = to_max9286_asd(asd)->source;
 	unsigned int index = to_index(priv, source);
 
 	source->sd = NULL;
@@ -569,23 +575,19 @@ static int max9286_v4l2_notifier_register(struct max9286_priv *priv)
 
 	for_each_source(priv, source) {
 		unsigned int i = to_index(priv, source);
+		struct v4l2_async_subdev *asd;
 
-		source->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
-		source->asd.match.fwnode = source->fwnode;
-
-		ret = v4l2_async_notifier_add_subdev(&priv->notifier,
-						     &source->asd);
-		if (ret) {
-			dev_err(dev, "Failed to add subdev for source %d", i);
+		asd = v4l2_async_notifier_add_fwnode_subdev(&priv->notifier,
+							    source->fwnode,
+							    sizeof(*asd));
+		if (IS_ERR(asd)) {
+			dev_err(dev, "Failed to add subdev for source %u: %ld",
+				i, PTR_ERR(asd));
 			v4l2_async_notifier_cleanup(&priv->notifier);
-			return ret;
+			return PTR_ERR(asd);
 		}
 
-		/*
-		 * Balance the reference counting handled through
-		 * v4l2_async_notifier_cleanup()
-		 */
-		fwnode_handle_get(source->fwnode);
+		to_max9286_asd(asd)->source = source;
 	}
 
 	priv->notifier.ops = &max9286_notify_ops;
-- 
Regards,

Laurent Pinchart


  parent reply	other threads:[~2020-08-11 21:00 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-11 20:59 [PATCH 0/5] media: Fix asd dynamic allocation Laurent Pinchart
2020-08-11 20:59 ` [PATCH 1/5] media: v4l2-async: Document asd allocation requirements Laurent Pinchart
2020-09-08  5:08   ` Laurent Pinchart
2020-09-16 15:44   ` Kieran Bingham
2020-08-11 20:59 ` [PATCH 2/5] media: rcar_drif: Fix fwnode reference leak when parsing DT Laurent Pinchart
2020-09-16 15:46   ` Kieran Bingham
2020-08-11 20:59 ` [PATCH 3/5] media: rcar_drif: Allocate v4l2_async_subdev dynamically Laurent Pinchart
2020-09-16 15:53   ` Kieran Bingham
2020-08-11 20:59 ` [PATCH 4/5] media: rcar-csi2: " Laurent Pinchart
2020-08-11 21:43   ` Niklas Söderlund
2020-08-11 22:14     ` Laurent Pinchart
2020-08-11 20:59 ` Laurent Pinchart [this message]
2020-09-08  5:08   ` [PATCH 5/5] media: i2c: max9286: " Laurent Pinchart
2020-09-14 12:55   ` Jacopo Mondi
2020-08-31 19:44 ` [PATCH 0/5] media: Fix asd dynamic allocation Ramesh Shanmugasundaram
2020-09-01 10:46   ` Chris Paterson
2020-09-10 15:53     ` Fabrizio Castro

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=20200811205939.19550-6-laurent.pinchart+renesas@ideasonboard.com \
    --to=laurent.pinchart+renesas@ideasonboard.com \
    --cc=jacopo+renesas@jmondi.org \
    --cc=kieran.bingham+renesas@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=niklas.soderlund+renesas@ragnatech.se \
    --cc=rashanmu@gmail.com \
    --cc=sakari.ailus@linux.intel.com \
    --cc=slongerbeam@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.