[PATCH] media: tc358743: release device_node in tc358743_probe_of()

[PATCH] media: tc358743: release device_node in tc358743_probe_of()

[Alexey Khoroshilov]

of_graph_get_next_endpoint() returns device_node with refcnt increased,
but these is no of_node_put() for it.

The patch adds one on error and normal paths.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 drivers/media/i2c/tc358743.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 393bbbbbaad7..44c41933415a 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1918,7 +1918,8 @@ static int tc358743_probe_of(struct tc358743_state *state)
 	endpoint = v4l2_fwnode_endpoint_alloc_parse(of_fwnode_handle(ep));
 	if (IS_ERR(endpoint)) {
 		dev_err(dev, "failed to parse endpoint\n");
-		return PTR_ERR(endpoint);
+		ret = PTR_ERR(endpoint);
+		goto put_node;
 	}
 
 	if (endpoint->bus_type != V4L2_MBUS_CSI2 ||
@@ -2013,6 +2014,8 @@ static int tc358743_probe_of(struct tc358743_state *state)
 	clk_disable_unprepare(refclk);
 free_endpoint:
 	v4l2_fwnode_endpoint_free(endpoint);
+put_node:
+	of_node_put(ep);
 	return ret;
 }
 #else
-- 
2.7.4

Re: [SIL2review] [PATCH] media: tc358743: release device_node in tc358743_probe_of()

[Nicholas Mc Guire]

On Sat, May 26, 2018 at 12:54:00AM +0300, Alexey Khoroshilov wrote:
> of_graph_get_next_endpoint() returns device_node with refcnt increased,
> but these is no of_node_put() for it.

I think this is correct - but would it not be simpler to do

   endpoint = v4l2_fwnode_endpoint_alloc_parse(of_fwnode_handle(ep));
   of_node_put(ep);
   if (IS_ERR(endpoint)) {
   ....

As the of_node_put(np) actually is unconditional anyway I think this
should be semantically equivalent.

> 
> The patch adds one on error and normal paths.
> 
> Found by Linux Driver Verification project (linuxtesting.org).
> 
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Reviewed-by: Nicholas Mc Guire <der.herr@hofr.at>
> ---
>  drivers/media/i2c/tc358743.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> index 393bbbbbaad7..44c41933415a 100644
> --- a/drivers/media/i2c/tc358743.c
> +++ b/drivers/media/i2c/tc358743.c
> @@ -1918,7 +1918,8 @@ static int tc358743_probe_of(struct tc358743_state *state)
>  	endpoint = v4l2_fwnode_endpoint_alloc_parse(of_fwnode_handle(ep));
>  	if (IS_ERR(endpoint)) {
>  		dev_err(dev, "failed to parse endpoint\n");
> -		return PTR_ERR(endpoint);
> +		ret = PTR_ERR(endpoint);
> +		goto put_node;
>  	}
>  
>  	if (endpoint->bus_type != V4L2_MBUS_CSI2 ||
> @@ -2013,6 +2014,8 @@ static int tc358743_probe_of(struct tc358743_state *state)
>  	clk_disable_unprepare(refclk);
>  free_endpoint:
>  	v4l2_fwnode_endpoint_free(endpoint);
> +put_node:
> +	of_node_put(ep);
>  	return ret;
>  }
>  #else
> -- 
> 2.7.4
> 
> _______________________________________________
> SIL2review mailing list
> SIL2review@lists.osadl.org
> https://lists.osadl.org/mailman/listinfo/sil2review

Re: [SIL2review] [PATCH] media: tc358743: release device_node in tc358743_probe_of()

[Alexey Khoroshilov]

On 26.05.2018 17:38, Nicholas Mc Guire wrote:
> On Sat, May 26, 2018 at 12:54:00AM +0300, Alexey Khoroshilov wrote:
>> of_graph_get_next_endpoint() returns device_node with refcnt increased,
>> but these is no of_node_put() for it.
> 
> I think this is correct - but would it not be simpler to do
> 
>    endpoint = v4l2_fwnode_endpoint_alloc_parse(of_fwnode_handle(ep));
>    of_node_put(ep);
>    if (IS_ERR(endpoint)) {
>    ....
> 
> As the of_node_put(np) actually is unconditional anyway I think this
> should be semantically equivalent.

You are right. But the same is true for
v4l2_fwnode_endpoint_free(endpoint);
that is already correctly handled by the driver.
So, I have preferred to follow the same pattern.


> 
>>
>> The patch adds one on error and normal paths.
>>
>> Found by Linux Driver Verification project (linuxtesting.org).
>>
>> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> Reviewed-by: Nicholas Mc Guire <der.herr@hofr.at>


Thank you,
Alexey