Message ID | 20200811205939.19550-5-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | media: Fix asd dynamic allocation | expand |
Hi Laurent, Thanks for your work. On 2020-08-11 23:59:38 +0300, Laurent Pinchart wrote: > v4l2_async_notifier_add_subdev() requires the asd to be allocated > dynamically, but the rcar-csi2 driver embeds it in the rcar_csi2 > 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. This patch conflicts with [1] which I think is a nicer solution to the problem, provided 1/2 of that series is palatable for everyone :-) 1. [PATCH 2/2] rcar-csi2: Use V4L2 async helpers to create the notifier > > Fixes: 769afd212b16 ("media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver") > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > drivers/media/platform/rcar-vin/rcar-csi2.c | 24 +++++++++------------ > 1 file changed, 10 insertions(+), 14 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c > index c6cc4f473a07..a16c492b3143 100644 > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c > @@ -362,7 +362,6 @@ struct rcar_csi2 { > struct media_pad pads[NR_OF_RCAR_CSI2_PAD]; > > struct v4l2_async_notifier notifier; > - struct v4l2_async_subdev asd; > struct v4l2_subdev *remote; > > struct v4l2_mbus_framefmt mf; > @@ -811,6 +810,8 @@ static int rcsi2_parse_v4l2(struct rcar_csi2 *priv, > > static int rcsi2_parse_dt(struct rcar_csi2 *priv) > { > + struct v4l2_async_subdev *asd; > + struct fwnode_handle *fwnode; > struct device_node *ep; > struct v4l2_fwnode_endpoint v4l2_ep = { .bus_type = 0 }; > int ret; > @@ -834,24 +835,19 @@ static int rcsi2_parse_dt(struct rcar_csi2 *priv) > return ret; > } > > - priv->asd.match.fwnode = > - fwnode_graph_get_remote_endpoint(of_fwnode_handle(ep)); > - priv->asd.match_type = V4L2_ASYNC_MATCH_FWNODE; > - > + fwnode = fwnode_graph_get_remote_endpoint(of_fwnode_handle(ep)); > of_node_put(ep); > > + dev_dbg(priv->dev, "Found '%pOF'\n", to_of_node(fwnode)); > + > v4l2_async_notifier_init(&priv->notifier); > - > - ret = v4l2_async_notifier_add_subdev(&priv->notifier, &priv->asd); > - if (ret) { > - fwnode_handle_put(priv->asd.match.fwnode); > - return ret; > - } > - > priv->notifier.ops = &rcar_csi2_notify_ops; > > - dev_dbg(priv->dev, "Found '%pOF'\n", > - to_of_node(priv->asd.match.fwnode)); > + asd = v4l2_async_notifier_add_fwnode_subdev(&priv->notifier, fwnode, > + sizeof(*asd)); > + fwnode_handle_put(fwnode); > + if (IS_ERR(asd)) > + return PTR_ERR(asd); > > ret = v4l2_async_subdev_notifier_register(&priv->subdev, > &priv->notifier); > -- > Regards, > > Laurent Pinchart >
Hi Niklas, On Tue, Aug 11, 2020 at 11:43:24PM +0200, Niklas Söderlund wrote: > On 2020-08-11 23:59:38 +0300, Laurent Pinchart wrote: > > v4l2_async_notifier_add_subdev() requires the asd to be allocated > > dynamically, but the rcar-csi2 driver embeds it in the rcar_csi2 > > 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. > > This patch conflicts with [1] which I think is a nicer solution to the > problem, provided 1/2 of that series is palatable for everyone :-) > > 1. [PATCH 2/2] rcar-csi2: Use V4L2 async helpers to create the notifier That looks better to me too. > > Fixes: 769afd212b16 ("media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver") > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > --- > > drivers/media/platform/rcar-vin/rcar-csi2.c | 24 +++++++++------------ > > 1 file changed, 10 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c > > index c6cc4f473a07..a16c492b3143 100644 > > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c > > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c > > @@ -362,7 +362,6 @@ struct rcar_csi2 { > > struct media_pad pads[NR_OF_RCAR_CSI2_PAD]; > > > > struct v4l2_async_notifier notifier; > > - struct v4l2_async_subdev asd; > > struct v4l2_subdev *remote; > > > > struct v4l2_mbus_framefmt mf; > > @@ -811,6 +810,8 @@ static int rcsi2_parse_v4l2(struct rcar_csi2 *priv, > > > > static int rcsi2_parse_dt(struct rcar_csi2 *priv) > > { > > + struct v4l2_async_subdev *asd; > > + struct fwnode_handle *fwnode; > > struct device_node *ep; > > struct v4l2_fwnode_endpoint v4l2_ep = { .bus_type = 0 }; > > int ret; > > @@ -834,24 +835,19 @@ static int rcsi2_parse_dt(struct rcar_csi2 *priv) > > return ret; > > } > > > > - priv->asd.match.fwnode = > > - fwnode_graph_get_remote_endpoint(of_fwnode_handle(ep)); > > - priv->asd.match_type = V4L2_ASYNC_MATCH_FWNODE; > > - > > + fwnode = fwnode_graph_get_remote_endpoint(of_fwnode_handle(ep)); > > of_node_put(ep); > > > > + dev_dbg(priv->dev, "Found '%pOF'\n", to_of_node(fwnode)); > > + > > v4l2_async_notifier_init(&priv->notifier); > > - > > - ret = v4l2_async_notifier_add_subdev(&priv->notifier, &priv->asd); > > - if (ret) { > > - fwnode_handle_put(priv->asd.match.fwnode); > > - return ret; > > - } > > - > > priv->notifier.ops = &rcar_csi2_notify_ops; > > > > - dev_dbg(priv->dev, "Found '%pOF'\n", > > - to_of_node(priv->asd.match.fwnode)); > > + asd = v4l2_async_notifier_add_fwnode_subdev(&priv->notifier, fwnode, > > + sizeof(*asd)); > > + fwnode_handle_put(fwnode); > > + if (IS_ERR(asd)) > > + return PTR_ERR(asd); > > > > ret = v4l2_async_subdev_notifier_register(&priv->subdev, > > &priv->notifier);
diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c index c6cc4f473a07..a16c492b3143 100644 --- a/drivers/media/platform/rcar-vin/rcar-csi2.c +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c @@ -362,7 +362,6 @@ struct rcar_csi2 { struct media_pad pads[NR_OF_RCAR_CSI2_PAD]; struct v4l2_async_notifier notifier; - struct v4l2_async_subdev asd; struct v4l2_subdev *remote; struct v4l2_mbus_framefmt mf; @@ -811,6 +810,8 @@ static int rcsi2_parse_v4l2(struct rcar_csi2 *priv, static int rcsi2_parse_dt(struct rcar_csi2 *priv) { + struct v4l2_async_subdev *asd; + struct fwnode_handle *fwnode; struct device_node *ep; struct v4l2_fwnode_endpoint v4l2_ep = { .bus_type = 0 }; int ret; @@ -834,24 +835,19 @@ static int rcsi2_parse_dt(struct rcar_csi2 *priv) return ret; } - priv->asd.match.fwnode = - fwnode_graph_get_remote_endpoint(of_fwnode_handle(ep)); - priv->asd.match_type = V4L2_ASYNC_MATCH_FWNODE; - + fwnode = fwnode_graph_get_remote_endpoint(of_fwnode_handle(ep)); of_node_put(ep); + dev_dbg(priv->dev, "Found '%pOF'\n", to_of_node(fwnode)); + v4l2_async_notifier_init(&priv->notifier); - - ret = v4l2_async_notifier_add_subdev(&priv->notifier, &priv->asd); - if (ret) { - fwnode_handle_put(priv->asd.match.fwnode); - return ret; - } - priv->notifier.ops = &rcar_csi2_notify_ops; - dev_dbg(priv->dev, "Found '%pOF'\n", - to_of_node(priv->asd.match.fwnode)); + asd = v4l2_async_notifier_add_fwnode_subdev(&priv->notifier, fwnode, + sizeof(*asd)); + fwnode_handle_put(fwnode); + if (IS_ERR(asd)) + return PTR_ERR(asd); ret = v4l2_async_subdev_notifier_register(&priv->subdev, &priv->notifier);
v4l2_async_notifier_add_subdev() requires the asd to be allocated dynamically, but the rcar-csi2 driver embeds it in the rcar_csi2 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. Fixes: 769afd212b16 ("media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver") Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- drivers/media/platform/rcar-vin/rcar-csi2.c | 24 +++++++++------------ 1 file changed, 10 insertions(+), 14 deletions(-)