* [PATCH] fsl/fman: add missing put_devcie() call in fman_port_probe()
@ 2020-10-31 10:54 Yu Kuai
2020-11-03 1:30 ` Jakub Kicinski
2020-11-03 11:23 ` [PATCH V2] " Yu Kuai
0 siblings, 2 replies; 9+ messages in thread
From: Yu Kuai @ 2020-10-31 10:54 UTC (permalink / raw)
To: madalin.bucur, davem, kuba, florinel.iordache
Cc: netdev, linux-kernel, yukuai3, yi.zhang
if of_find_device_by_node() succeed, fman_port_probe() doesn't have a
corresponding put_device(). Thus add jump target to fix the exception
handling for this function implementation.
Fixes: 0572054617f3 ("fsl/fman: fix dereference null return value")
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
drivers/net/ethernet/freescale/fman/fman_port.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fman/fman_port.c b/drivers/net/ethernet/freescale/fman/fman_port.c
index d9baac0dbc7d..576ce6df3fce 100644
--- a/drivers/net/ethernet/freescale/fman/fman_port.c
+++ b/drivers/net/ethernet/freescale/fman/fman_port.c
@@ -1799,13 +1799,13 @@ static int fman_port_probe(struct platform_device *of_dev)
of_node_put(fm_node);
if (!fm_pdev) {
err = -EINVAL;
- goto return_err;
+ goto put_device;
}
fman = dev_get_drvdata(&fm_pdev->dev);
if (!fman) {
err = -EINVAL;
- goto return_err;
+ goto put_device;
}
err = of_property_read_u32(port_node, "cell-index", &val);
@@ -1813,7 +1813,7 @@ static int fman_port_probe(struct platform_device *of_dev)
dev_err(port->dev, "%s: reading cell-index for %pOF failed\n",
__func__, port_node);
err = -EINVAL;
- goto return_err;
+ goto put_device;
}
port_id = (u8)val;
port->dts_params.id = port_id;
@@ -1847,7 +1847,7 @@ static int fman_port_probe(struct platform_device *of_dev)
} else {
dev_err(port->dev, "%s: Illegal port type\n", __func__);
err = -EINVAL;
- goto return_err;
+ goto put_device;
}
port->dts_params.type = port_type;
@@ -1861,7 +1861,7 @@ static int fman_port_probe(struct platform_device *of_dev)
dev_err(port->dev, "%s: incorrect qman-channel-id\n",
__func__);
err = -EINVAL;
- goto return_err;
+ goto put_device;
}
port->dts_params.qman_channel_id = qman_channel_id;
}
@@ -1871,7 +1871,7 @@ static int fman_port_probe(struct platform_device *of_dev)
dev_err(port->dev, "%s: of_address_to_resource() failed\n",
__func__);
err = -ENOMEM;
- goto return_err;
+ goto put_device;
}
port->dts_params.fman = fman;
@@ -1898,6 +1898,8 @@ static int fman_port_probe(struct platform_device *of_dev)
return_err:
of_node_put(port_node);
+put_device:
+ put_device(&fm_pdev->dev);
free_port:
kfree(port);
return err;
--
2.25.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] fsl/fman: add missing put_devcie() call in fman_port_probe()
2020-10-31 10:54 [PATCH] fsl/fman: add missing put_devcie() call in fman_port_probe() Yu Kuai
@ 2020-11-03 1:30 ` Jakub Kicinski
2020-11-03 11:09 ` yukuai (C)
2020-11-03 11:23 ` [PATCH V2] " Yu Kuai
1 sibling, 1 reply; 9+ messages in thread
From: Jakub Kicinski @ 2020-11-03 1:30 UTC (permalink / raw)
To: Yu Kuai
Cc: madalin.bucur, davem, florinel.iordache, netdev, linux-kernel, yi.zhang
On Sat, 31 Oct 2020 18:54:18 +0800 Yu Kuai wrote:
> if of_find_device_by_node() succeed, fman_port_probe() doesn't have a
> corresponding put_device(). Thus add jump target to fix the exception
> handling for this function implementation.
>
> Fixes: 0572054617f3 ("fsl/fman: fix dereference null return value")
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> diff --git a/drivers/net/ethernet/freescale/fman/fman_port.c b/drivers/net/ethernet/freescale/fman/fman_port.c
> index d9baac0dbc7d..576ce6df3fce 100644
> --- a/drivers/net/ethernet/freescale/fman/fman_port.c
> +++ b/drivers/net/ethernet/freescale/fman/fman_port.c
> @@ -1799,13 +1799,13 @@ static int fman_port_probe(struct platform_device *of_dev)
> of_node_put(fm_node);
> if (!fm_pdev) {
> err = -EINVAL;
> - goto return_err;
> + goto put_device;
> }
> @@ -1898,6 +1898,8 @@ static int fman_port_probe(struct platform_device *of_dev)
>
> return_err:
> of_node_put(port_node);
> +put_device:
> + put_device(&fm_pdev->dev);
> free_port:
> kfree(port);
> return err;
This does not look right. You're jumping to put_device() when fm_pdev
is NULL?
The order of error handling should be the reverse of the order of
execution of the function.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fsl/fman: add missing put_devcie() call in fman_port_probe()
2020-11-03 1:30 ` Jakub Kicinski
@ 2020-11-03 11:09 ` yukuai (C)
0 siblings, 0 replies; 9+ messages in thread
From: yukuai (C) @ 2020-11-03 11:09 UTC (permalink / raw)
To: Jakub Kicinski
Cc: madalin.bucur, davem, florinel.iordache, netdev, linux-kernel, yi.zhang
On 2020/11/03 9:30, Jakub Kicinski wrote:
> On Sat, 31 Oct 2020 18:54:18 +0800 Yu Kuai wrote:
>> if of_find_device_by_node() succeed, fman_port_probe() doesn't have a
>> corresponding put_device(). Thus add jump target to fix the exception
>> handling for this function implementation.
>>
>> Fixes: 0572054617f3 ("fsl/fman: fix dereference null return value")
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>
>> diff --git a/drivers/net/ethernet/freescale/fman/fman_port.c b/drivers/net/ethernet/freescale/fman/fman_port.c
>> index d9baac0dbc7d..576ce6df3fce 100644
>> --- a/drivers/net/ethernet/freescale/fman/fman_port.c
>> +++ b/drivers/net/ethernet/freescale/fman/fman_port.c
>> @@ -1799,13 +1799,13 @@ static int fman_port_probe(struct platform_device *of_dev)
>> of_node_put(fm_node);
>> if (!fm_pdev) {
>> err = -EINVAL;
>> - goto return_err;
>> + goto put_device;
>> }
>
>> @@ -1898,6 +1898,8 @@ static int fman_port_probe(struct platform_device *of_dev)
>>
>> return_err:
>> of_node_put(port_node);
>> +put_device:
>> + put_device(&fm_pdev->dev);
>> free_port:
>> kfree(port);
>> return err;
>
> This does not look right. You're jumping to put_device() when fm_pdev
> is NULL?
>
Hi,
oops, it's a silly mistake. Will fix it in V2 patch.
Thanks,
Yu Kuai
> The order of error handling should be the reverse of the order of
> execution of the function.
> .
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V2] fsl/fman: add missing put_devcie() call in fman_port_probe()
2020-10-31 10:54 [PATCH] fsl/fman: add missing put_devcie() call in fman_port_probe() Yu Kuai
2020-11-03 1:30 ` Jakub Kicinski
@ 2020-11-03 11:23 ` Yu Kuai
2020-11-05 1:31 ` Jakub Kicinski
2020-11-07 9:09 ` [PATCH V3] " Yu Kuai
1 sibling, 2 replies; 9+ messages in thread
From: Yu Kuai @ 2020-11-03 11:23 UTC (permalink / raw)
To: madalin.bucur, davem, kuba, florinel.iordache
Cc: netdev, linux-kernel, yukuai3, yi.zhang
if of_find_device_by_node() succeed, fman_port_probe() doesn't have a
corresponding put_device(). Thus add jump target to fix the exception
handling for this function implementation.
Fixes: 0572054617f3 ("fsl/fman: fix dereference null return value")
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
.../net/ethernet/freescale/fman/fman_port.c | 23 +++++++++----------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fman/fman_port.c b/drivers/net/ethernet/freescale/fman/fman_port.c
index d9baac0dbc7d..4ae5d844d1f5 100644
--- a/drivers/net/ethernet/freescale/fman/fman_port.c
+++ b/drivers/net/ethernet/freescale/fman/fman_port.c
@@ -1792,20 +1792,21 @@ static int fman_port_probe(struct platform_device *of_dev)
if (!fm_node) {
dev_err(port->dev, "%s: of_get_parent() failed\n", __func__);
err = -ENODEV;
- goto return_err;
+ goto free_port;
}
+ of_node_put(port_node);
fm_pdev = of_find_device_by_node(fm_node);
of_node_put(fm_node);
if (!fm_pdev) {
err = -EINVAL;
- goto return_err;
+ goto free_port;
}
fman = dev_get_drvdata(&fm_pdev->dev);
if (!fman) {
err = -EINVAL;
- goto return_err;
+ goto put_device;
}
err = of_property_read_u32(port_node, "cell-index", &val);
@@ -1813,7 +1814,7 @@ static int fman_port_probe(struct platform_device *of_dev)
dev_err(port->dev, "%s: reading cell-index for %pOF failed\n",
__func__, port_node);
err = -EINVAL;
- goto return_err;
+ goto put_device;
}
port_id = (u8)val;
port->dts_params.id = port_id;
@@ -1847,7 +1848,7 @@ static int fman_port_probe(struct platform_device *of_dev)
} else {
dev_err(port->dev, "%s: Illegal port type\n", __func__);
err = -EINVAL;
- goto return_err;
+ goto put_device;
}
port->dts_params.type = port_type;
@@ -1861,7 +1862,7 @@ static int fman_port_probe(struct platform_device *of_dev)
dev_err(port->dev, "%s: incorrect qman-channel-id\n",
__func__);
err = -EINVAL;
- goto return_err;
+ goto put_device;
}
port->dts_params.qman_channel_id = qman_channel_id;
}
@@ -1871,20 +1872,18 @@ static int fman_port_probe(struct platform_device *of_dev)
dev_err(port->dev, "%s: of_address_to_resource() failed\n",
__func__);
err = -ENOMEM;
- goto return_err;
+ goto put_device;
}
port->dts_params.fman = fman;
- of_node_put(port_node);
-
dev_res = __devm_request_region(port->dev, &res, res.start,
resource_size(&res), "fman-port");
if (!dev_res) {
dev_err(port->dev, "%s: __devm_request_region() failed\n",
__func__);
err = -EINVAL;
- goto free_port;
+ goto put_device;
}
port->dts_params.base_addr = devm_ioremap(port->dev, res.start,
@@ -1896,8 +1895,8 @@ static int fman_port_probe(struct platform_device *of_dev)
return 0;
-return_err:
- of_node_put(port_node);
+put_device:
+ put_device(&fm_pdev->dev);
free_port:
kfree(port);
return err;
--
2.25.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V2] fsl/fman: add missing put_devcie() call in fman_port_probe()
2020-11-03 11:23 ` [PATCH V2] " Yu Kuai
@ 2020-11-05 1:31 ` Jakub Kicinski
2020-11-06 21:59 ` Madalin Bucur (OSS)
2020-11-07 9:09 ` [PATCH V3] " Yu Kuai
1 sibling, 1 reply; 9+ messages in thread
From: Jakub Kicinski @ 2020-11-05 1:31 UTC (permalink / raw)
To: Yu Kuai
Cc: madalin.bucur, davem, florinel.iordache, netdev, linux-kernel, yi.zhang
On Tue, 3 Nov 2020 19:23:23 +0800 Yu Kuai wrote:
> --- a/drivers/net/ethernet/freescale/fman/fman_port.c
> +++ b/drivers/net/ethernet/freescale/fman/fman_port.c
> @@ -1792,20 +1792,21 @@ static int fman_port_probe(struct platform_device *of_dev)
> if (!fm_node) {
> dev_err(port->dev, "%s: of_get_parent() failed\n", __func__);
> err = -ENODEV;
> - goto return_err;
> + goto free_port;
> }
>
> + of_node_put(port_node);
> fm_pdev = of_find_device_by_node(fm_node);
> of_node_put(fm_node);
> if (!fm_pdev) {
> err = -EINVAL;
> - goto return_err;
> + goto free_port;
> }
This is not right either. I just asked you fix up the order of the
error path, not move the of_node_put() in the body of the function.
Now you're releasing the reference on the object and still use it after.
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH V2] fsl/fman: add missing put_devcie() call in fman_port_probe()
2020-11-05 1:31 ` Jakub Kicinski
@ 2020-11-06 21:59 ` Madalin Bucur (OSS)
0 siblings, 0 replies; 9+ messages in thread
From: Madalin Bucur (OSS) @ 2020-11-06 21:59 UTC (permalink / raw)
To: Yu Kuai
Cc: Jakub Kicinski, davem, Florinel Iordache, netdev, linux-kernel, yi.zhang
> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: 05 November 2020 03:31
> To: Yu Kuai <yukuai3@huawei.com>
> Cc: Madalin Bucur <madalin.bucur@nxp.com>; davem@davemloft.net; Florinel
> Iordache <florinel.iordache@nxp.com>; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; yi.zhang@huawei.com
> Subject: Re: [PATCH V2] fsl/fman: add missing put_devcie() call in
> fman_port_probe()
>
> On Tue, 3 Nov 2020 19:23:23 +0800 Yu Kuai wrote:
> > --- a/drivers/net/ethernet/freescale/fman/fman_port.c
> > +++ b/drivers/net/ethernet/freescale/fman/fman_port.c
> > @@ -1792,20 +1792,21 @@ static int fman_port_probe(struct
> platform_device *of_dev)
> > if (!fm_node) {
> > dev_err(port->dev, "%s: of_get_parent() failed\n", __func__);
> > err = -ENODEV;
> > - goto return_err;
> > + goto free_port;
> > }
> >
> > + of_node_put(port_node);
> > fm_pdev = of_find_device_by_node(fm_node);
> > of_node_put(fm_node);
> > if (!fm_pdev) {
> > err = -EINVAL;
> > - goto return_err;
> > + goto free_port;
> > }
>
> This is not right either. I just asked you fix up the order of the
> error path, not move the of_node_put() in the body of the function.
>
> Now you're releasing the reference on the object and still use it after.
If you manage to put together a v3, please also address the typo in the
subject (put_devcie).
Madalin
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V3] fsl/fman: add missing put_devcie() call in fman_port_probe()
2020-11-03 11:23 ` [PATCH V2] " Yu Kuai
2020-11-05 1:31 ` Jakub Kicinski
@ 2020-11-07 9:09 ` Yu Kuai
2020-11-07 22:09 ` Jakub Kicinski
1 sibling, 1 reply; 9+ messages in thread
From: Yu Kuai @ 2020-11-07 9:09 UTC (permalink / raw)
To: madalin.bucur, davem, kuba, florinel.iordache
Cc: netdev, linux-kernel, yukuai3, yi.zhang
if of_find_device_by_node() succeed, fman_port_probe() doesn't have a
corresponding put_device(). Thus add jump target to fix the exception
handling for this function implementation.
Fixes: 0572054617f3 ("fsl/fman: fix dereference null return value")
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
Changes in V3:
- move of_node_put(port_node) backward, so that error handling can be the
reverse of the order of execution.
- rename retur_err to put_node
.../net/ethernet/freescale/fman/fman_port.c | 23 ++++++++++---------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fman/fman_port.c b/drivers/net/ethernet/freescale/fman/fman_port.c
index d9baac0dbc7d..9790e483241b 100644
--- a/drivers/net/ethernet/freescale/fman/fman_port.c
+++ b/drivers/net/ethernet/freescale/fman/fman_port.c
@@ -1792,20 +1792,20 @@ static int fman_port_probe(struct platform_device *of_dev)
if (!fm_node) {
dev_err(port->dev, "%s: of_get_parent() failed\n", __func__);
err = -ENODEV;
- goto return_err;
+ goto free_port;
}
fm_pdev = of_find_device_by_node(fm_node);
of_node_put(fm_node);
if (!fm_pdev) {
err = -EINVAL;
- goto return_err;
+ goto put_node;
}
fman = dev_get_drvdata(&fm_pdev->dev);
if (!fman) {
err = -EINVAL;
- goto return_err;
+ goto put_device;
}
err = of_property_read_u32(port_node, "cell-index", &val);
@@ -1813,7 +1813,7 @@ static int fman_port_probe(struct platform_device *of_dev)
dev_err(port->dev, "%s: reading cell-index for %pOF failed\n",
__func__, port_node);
err = -EINVAL;
- goto return_err;
+ goto put_device;
}
port_id = (u8)val;
port->dts_params.id = port_id;
@@ -1847,7 +1847,7 @@ static int fman_port_probe(struct platform_device *of_dev)
} else {
dev_err(port->dev, "%s: Illegal port type\n", __func__);
err = -EINVAL;
- goto return_err;
+ goto put_device;
}
port->dts_params.type = port_type;
@@ -1861,7 +1861,7 @@ static int fman_port_probe(struct platform_device *of_dev)
dev_err(port->dev, "%s: incorrect qman-channel-id\n",
__func__);
err = -EINVAL;
- goto return_err;
+ goto put_device;
}
port->dts_params.qman_channel_id = qman_channel_id;
}
@@ -1871,22 +1871,21 @@ static int fman_port_probe(struct platform_device *of_dev)
dev_err(port->dev, "%s: of_address_to_resource() failed\n",
__func__);
err = -ENOMEM;
- goto return_err;
+ goto put_device;
}
port->dts_params.fman = fman;
- of_node_put(port_node);
-
dev_res = __devm_request_region(port->dev, &res, res.start,
resource_size(&res), "fman-port");
if (!dev_res) {
dev_err(port->dev, "%s: __devm_request_region() failed\n",
__func__);
err = -EINVAL;
- goto free_port;
+ goto put_device;
}
+ of_node_put(port_node);
port->dts_params.base_addr = devm_ioremap(port->dev, res.start,
resource_size(&res));
if (!port->dts_params.base_addr)
@@ -1896,7 +1895,9 @@ static int fman_port_probe(struct platform_device *of_dev)
return 0;
-return_err:
+put_device:
+ put_device(&fm_pdev->dev);
+put_node:
of_node_put(port_node);
free_port:
kfree(port);
--
2.25.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V3] fsl/fman: add missing put_devcie() call in fman_port_probe()
2020-11-07 9:09 ` [PATCH V3] " Yu Kuai
@ 2020-11-07 22:09 ` Jakub Kicinski
2020-11-09 13:11 ` yukuai (C)
0 siblings, 1 reply; 9+ messages in thread
From: Jakub Kicinski @ 2020-11-07 22:09 UTC (permalink / raw)
To: Yu Kuai
Cc: madalin.bucur, davem, florinel.iordache, netdev, linux-kernel, yi.zhang
On Sat, 7 Nov 2020 17:09:25 +0800 Yu Kuai wrote:
> if of_find_device_by_node() succeed, fman_port_probe() doesn't have a
> corresponding put_device(). Thus add jump target to fix the exception
> handling for this function implementation.
>
> Fixes: 0572054617f3 ("fsl/fman: fix dereference null return value")
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> @@ -1792,20 +1792,20 @@ static int fman_port_probe(struct platform_device *of_dev)
> if (!fm_node) {
> dev_err(port->dev, "%s: of_get_parent() failed\n", __func__);
> err = -ENODEV;
> - goto return_err;
> + goto free_port;
And now you no longer put port_node if jumping from here...
Also does the reference to put_device() not have to be released when
this function succeeds?
> }
> @@ -1896,7 +1895,9 @@ static int fman_port_probe(struct platform_device *of_dev)
>
> return 0;
>
> -return_err:
> +put_device:
> + put_device(&fm_pdev->dev);
> +put_node:
> of_node_put(port_node);
> free_port:
> kfree(port);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V3] fsl/fman: add missing put_devcie() call in fman_port_probe()
2020-11-07 22:09 ` Jakub Kicinski
@ 2020-11-09 13:11 ` yukuai (C)
0 siblings, 0 replies; 9+ messages in thread
From: yukuai (C) @ 2020-11-09 13:11 UTC (permalink / raw)
To: Jakub Kicinski
Cc: madalin.bucur, davem, florinel.iordache, netdev, linux-kernel, yi.zhang
在 2020/11/08 6:09, Jakub Kicinski 写道:
> On Sat, 7 Nov 2020 17:09:25 +0800 Yu Kuai wrote:
>> if of_find_device_by_node() succeed, fman_port_probe() doesn't have a
>> corresponding put_device(). Thus add jump target to fix the exception
>> handling for this function implementation.
>>
>> Fixes: 0572054617f3 ("fsl/fman: fix dereference null return value")
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>
>> @@ -1792,20 +1792,20 @@ static int fman_port_probe(struct platform_device *of_dev)
>> if (!fm_node) {
>> dev_err(port->dev, "%s: of_get_parent() failed\n", __func__);
>> err = -ENODEV;
>> - goto return_err;
>> + goto free_port;
>
> And now you no longer put port_node if jumping from here...
Sincerely apologize for that stupid mistake...
>
> Also does the reference to put_device() not have to be released when
> this function succeeds?
>
I'm not sure about that, since fman_port_driver doesn't define other
interface, maybe it reasonable to release it here.
>> }
>
>> @@ -1896,7 +1895,9 @@ static int fman_port_probe(struct platform_device *of_dev)
>>
>> return 0;
>>
>> -return_err:
>> +put_device:
>> + put_device(&fm_pdev->dev);
>> +put_node:
>> of_node_put(port_node);
>> free_port:
>> kfree(port);
>
> .
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-11-09 13:11 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-31 10:54 [PATCH] fsl/fman: add missing put_devcie() call in fman_port_probe() Yu Kuai
2020-11-03 1:30 ` Jakub Kicinski
2020-11-03 11:09 ` yukuai (C)
2020-11-03 11:23 ` [PATCH V2] " Yu Kuai
2020-11-05 1:31 ` Jakub Kicinski
2020-11-06 21:59 ` Madalin Bucur (OSS)
2020-11-07 9:09 ` [PATCH V3] " Yu Kuai
2020-11-07 22:09 ` Jakub Kicinski
2020-11-09 13:11 ` yukuai (C)
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).