diff mbox series

fsl/fman: add missing put_devcie() call in fman_port_probe()

Message ID 20201031105418.2304011-1-yukuai3@huawei.com
State Changes Requested
Delegated to: David Miller
Headers show
Series fsl/fman: add missing put_devcie() call in fman_port_probe() | expand

Checks

Context Check Description
jkicinski/cover_letter success Link
jkicinski/fixes_present success Link
jkicinski/patch_count success Link
jkicinski/tree_selection success Guessed tree name to be net-next
jkicinski/subject_prefix warning Target tree name not specified in the subject
jkicinski/source_inline success Was 0 now: 0
jkicinski/verify_signedoff success Link
jkicinski/module_param success Was 0 now: 0
jkicinski/build_32bit success Errors and warnings before: 0 this patch: 0
jkicinski/kdoc success Errors and warnings before: 0 this patch: 0
jkicinski/verify_fixes success Link
jkicinski/checkpatch success total: 0 errors, 0 warnings, 0 checks, 55 lines checked
jkicinski/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
jkicinski/header_inline success Link
jkicinski/stable success Stable not CCed

Commit Message

yukuai (C) Oct. 31, 2020, 10:54 a.m. UTC
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(-)

Comments

Jakub Kicinski Nov. 3, 2020, 1:30 a.m. UTC | #1
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.
yukuai (C) Nov. 3, 2020, 11:09 a.m. UTC | #2
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.
> .
>
diff mbox series

Patch

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;