All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fsi: Fix error handling in fsi_master_register() and its callers
@ 2022-04-21 16:02 Christophe JAILLET
  2022-04-22  6:33   ` Christophe JAILLET
  0 siblings, 1 reply; 3+ messages in thread
From: Christophe JAILLET @ 2022-04-21 16:02 UTC (permalink / raw)
  To: Jeremy Kerr, Joel Stanley, Alistar Popple, Eddie James
  Cc: linux-kernel, kernel-janitors, Christophe JAILLET, linux-fsi

fsi_master_register() calls device_register().
When device_register() fails, put_device() still needs to be called.

Up to now, there is no put_device() in fsi_master_register().
Some callers of fsi_master_register() have it, some have not.
 - fsi_master_acf_probe() call put_device() if fsi_master_register() fails
 - fsi_master_gpio_probe() call put_device() if fsi_master_register() fails
 - fsi_master_aspeed_probe() doesn't
 - hub_master_probe() doesn't

In order to fix it and be consistent with the different callers, add the
missing put_device() in the error handling path of fsi_master_register()
and remove it from 2 callers that were handling it by themselves.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
This patch is speculative and compile tested only.
Review with care.

Another alternative would be to add the put_device() call in
fsi_master_aspeed_probe() and hub_master_probe() if it makes more sense.

Having one or more Fixes tag is a bit hard because of the relations and
log history of the 5 files involved in this bug fix.
---
 drivers/fsi/fsi-core.c          | 1 +
 drivers/fsi/fsi-master-ast-cf.c | 1 -
 drivers/fsi/fsi-master-gpio.c   | 1 -
 3 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 3a7b78e36701..640692e5400f 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -1319,6 +1319,7 @@ int fsi_master_register(struct fsi_master *master)
 
 	rc = device_register(&master->dev);
 	if (rc) {
+		put_device(&master->dev);
 		ida_simple_remove(&master_ida, master->idx);
 		return rc;
 	}
diff --git a/drivers/fsi/fsi-master-ast-cf.c b/drivers/fsi/fsi-master-ast-cf.c
index 24292acdbaf8..dde63d703ea1 100644
--- a/drivers/fsi/fsi-master-ast-cf.c
+++ b/drivers/fsi/fsi-master-ast-cf.c
@@ -1395,7 +1395,6 @@ static int fsi_master_acf_probe(struct platform_device *pdev)
 		return 0;
 
 	device_remove_file(master->dev, &dev_attr_external_mode);
-	put_device(&master->master.dev);
 	return rc;
 
  stop_copro:
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index 7d5f29b4b595..1ae3c164d7fd 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -856,7 +856,6 @@ static int fsi_master_gpio_probe(struct platform_device *pdev)
 	rc = fsi_master_register(&master->master);
 	if (rc) {
 		device_remove_file(&pdev->dev, &dev_attr_external_mode);
-		put_device(&master->master.dev);
 		return rc;
 	}
 	return 0;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] fsi: Fix error handling in fsi_master_register() and its callers
@ 2022-04-22  6:33   ` Christophe JAILLET
  0 siblings, 0 replies; 3+ messages in thread
From: Christophe JAILLET @ 2022-04-22  6:33 UTC (permalink / raw)
  To: Jeremy Kerr, Joel Stanley, Alistar Popple, Eddie James
  Cc: linux-kernel, kernel-janitors, linux-fsi

Le 21/04/2022 à 18:02, Christophe JAILLET a écrit :
> fsi_master_register() calls device_register().
> When device_register() fails, put_device() still needs to be called.
> 
> Up to now, there is no put_device() in fsi_master_register().
> Some callers of fsi_master_register() have it, some have not.
>   - fsi_master_acf_probe() call put_device() if fsi_master_register() fails
>   - fsi_master_gpio_probe() call put_device() if fsi_master_register() fails
>   - fsi_master_aspeed_probe() doesn't
>   - hub_master_probe() doesn't
> 
> In order to fix it and be consistent with the different callers, add the
> missing put_device() in the error handling path of fsi_master_register()
> and remove it from 2 callers that were handling it by themselves.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> This patch is speculative and compile tested only.
> Review with care.
> 
> Another alternative would be to add the put_device() call in
> fsi_master_aspeed_probe() and hub_master_probe() if it makes more sense.
> 
> Having one or more Fixes tag is a bit hard because of the relations and
> log history of the 5 files involved in this bug fix.
> ---
>   drivers/fsi/fsi-core.c          | 1 +
>   drivers/fsi/fsi-master-ast-cf.c | 1 -
>   drivers/fsi/fsi-master-gpio.c   | 1 -
>   3 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index 3a7b78e36701..640692e5400f 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -1319,6 +1319,7 @@ int fsi_master_register(struct fsi_master *master)
>   
>   	rc = device_register(&master->dev);
>   	if (rc) {
> +		put_device(&master->dev);
>   		ida_simple_remove(&master_ida, master->idx);
>   		return rc;
>   	}
> diff --git a/drivers/fsi/fsi-master-ast-cf.c b/drivers/fsi/fsi-master-ast-cf.c
> index 24292acdbaf8..dde63d703ea1 100644
> --- a/drivers/fsi/fsi-master-ast-cf.c
> +++ b/drivers/fsi/fsi-master-ast-cf.c
> @@ -1395,7 +1395,6 @@ static int fsi_master_acf_probe(struct platform_device *pdev)
>   		return 0;
>   
>   	device_remove_file(master->dev, &dev_attr_external_mode);
> -	put_device(&master->master.dev);
>   	return rc;
>   
>    stop_copro:
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> index 7d5f29b4b595..1ae3c164d7fd 100644
> --- a/drivers/fsi/fsi-master-gpio.c
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -856,7 +856,6 @@ static int fsi_master_gpio_probe(struct platform_device *pdev)
>   	rc = fsi_master_register(&master->master);
>   	if (rc) {
>   		device_remove_file(&pdev->dev, &dev_attr_external_mode);
> -		put_device(&master->master.dev);
>   		return rc;
>   	}
>   	return 0;

NACK

After additional reading of the code, it looks good to me as-is.
Usage in fsi_master_aspeed_probe() and hub_master_probe() seem to have 
no release function.

CJ

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] fsi: Fix error handling in fsi_master_register() and its callers
@ 2022-04-22  6:33   ` Christophe JAILLET
  0 siblings, 0 replies; 3+ messages in thread
From: Christophe JAILLET @ 2022-04-22  6:33 UTC (permalink / raw)
  To: linux-kernel; +Cc: kernel-janitors

Le 21/04/2022 à 18:02, Christophe JAILLET a écrit :
> fsi_master_register() calls device_register().
> When device_register() fails, put_device() still needs to be called.
> 
> Up to now, there is no put_device() in fsi_master_register().
> Some callers of fsi_master_register() have it, some have not.
>   - fsi_master_acf_probe() call put_device() if fsi_master_register() fails
>   - fsi_master_gpio_probe() call put_device() if fsi_master_register() fails
>   - fsi_master_aspeed_probe() doesn't
>   - hub_master_probe() doesn't
> 
> In order to fix it and be consistent with the different callers, add the
> missing put_device() in the error handling path of fsi_master_register()
> and remove it from 2 callers that were handling it by themselves.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> This patch is speculative and compile tested only.
> Review with care.
> 
> Another alternative would be to add the put_device() call in
> fsi_master_aspeed_probe() and hub_master_probe() if it makes more sense.
> 
> Having one or more Fixes tag is a bit hard because of the relations and
> log history of the 5 files involved in this bug fix.
> ---
>   drivers/fsi/fsi-core.c          | 1 +
>   drivers/fsi/fsi-master-ast-cf.c | 1 -
>   drivers/fsi/fsi-master-gpio.c   | 1 -
>   3 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index 3a7b78e36701..640692e5400f 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -1319,6 +1319,7 @@ int fsi_master_register(struct fsi_master *master)
>   
>   	rc = device_register(&master->dev);
>   	if (rc) {
> +		put_device(&master->dev);
>   		ida_simple_remove(&master_ida, master->idx);
>   		return rc;
>   	}
> diff --git a/drivers/fsi/fsi-master-ast-cf.c b/drivers/fsi/fsi-master-ast-cf.c
> index 24292acdbaf8..dde63d703ea1 100644
> --- a/drivers/fsi/fsi-master-ast-cf.c
> +++ b/drivers/fsi/fsi-master-ast-cf.c
> @@ -1395,7 +1395,6 @@ static int fsi_master_acf_probe(struct platform_device *pdev)
>   		return 0;
>   
>   	device_remove_file(master->dev, &dev_attr_external_mode);
> -	put_device(&master->master.dev);
>   	return rc;
>   
>    stop_copro:
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> index 7d5f29b4b595..1ae3c164d7fd 100644
> --- a/drivers/fsi/fsi-master-gpio.c
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -856,7 +856,6 @@ static int fsi_master_gpio_probe(struct platform_device *pdev)
>   	rc = fsi_master_register(&master->master);
>   	if (rc) {
>   		device_remove_file(&pdev->dev, &dev_attr_external_mode);
> -		put_device(&master->master.dev);
>   		return rc;
>   	}
>   	return 0;

NACK

After additional reading of the code, it looks good to me as-is.
Usage in fsi_master_aspeed_probe() and hub_master_probe() seem to have 
no release function.

CJ


^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-04-22  6:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-21 16:02 [PATCH] fsi: Fix error handling in fsi_master_register() and its callers Christophe JAILLET
2022-04-22  6:33 ` Christophe JAILLET
2022-04-22  6:33   ` Christophe JAILLET

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.