linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] phy: core: check whether ops callback function is assigned
       [not found] <CGME20161222094224epcas1p49349afba1cf8468d350b66ca29e8590e@epcas1p4.samsung.com>
@ 2016-12-22  9:42 ` Jaehoon Chung
  2016-12-27  8:31   ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 3+ messages in thread
From: Jaehoon Chung @ 2016-12-22  9:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: kishon, Jaehoon Chung

If some ops-> callback function are not assigend, then it should do the
unexpect behavior.
To prevent the potential NULL pointer dereference, check the each
callback functions before doing operation.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/phy/phy-core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index a268f4d6f3e9..e4eb4431c8a4 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -219,7 +219,7 @@ int phy_init(struct phy *phy)
 {
 	int ret;
 
-	if (!phy)
+	if (!phy || !phy->ops->init)
 		return 0;
 
 	ret = phy_pm_runtime_get_sync(phy);
@@ -248,7 +248,7 @@ int phy_exit(struct phy *phy)
 {
 	int ret;
 
-	if (!phy)
+	if (!phy || !phy->ops->exit)
 		return 0;
 
 	ret = phy_pm_runtime_get_sync(phy);
@@ -277,7 +277,7 @@ int phy_power_on(struct phy *phy)
 {
 	int ret = 0;
 
-	if (!phy)
+	if (!phy || !phy->ops->power_on)
 		goto out;
 
 	if (phy->pwr) {
@@ -319,7 +319,7 @@ int phy_power_off(struct phy *phy)
 {
 	int ret;
 
-	if (!phy)
+	if (!phy || !phy->ops->power_off)
 		return 0;
 
 	mutex_lock(&phy->mutex);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] phy: core: check whether ops callback function is assigned
  2016-12-22  9:42 ` [PATCH] phy: core: check whether ops callback function is assigned Jaehoon Chung
@ 2016-12-27  8:31   ` Kishon Vijay Abraham I
  2016-12-27  8:37     ` Jaehoon Chung
  0 siblings, 1 reply; 3+ messages in thread
From: Kishon Vijay Abraham I @ 2016-12-27  8:31 UTC (permalink / raw)
  To: Jaehoon Chung, linux-kernel

Hi,

On Thursday 22 December 2016 03:12 PM, Jaehoon Chung wrote:
> If some ops-> callback function are not assigend, then it should do the
> unexpect behavior.
> To prevent the potential NULL pointer dereference, check the each
> callback functions before doing operation.

The call backs checks are done after the mutex. Moreover even if the call backs
are not assigned, the user can call the phy ops for doing pm_runtime.

Thanks
Kishon

> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/phy/phy-core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index a268f4d6f3e9..e4eb4431c8a4 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -219,7 +219,7 @@ int phy_init(struct phy *phy)
>  {
>  	int ret;
>  
> -	if (!phy)
> +	if (!phy || !phy->ops->init)
>  		return 0;
>  
>  	ret = phy_pm_runtime_get_sync(phy);
> @@ -248,7 +248,7 @@ int phy_exit(struct phy *phy)
>  {
>  	int ret;
>  
> -	if (!phy)
> +	if (!phy || !phy->ops->exit)
>  		return 0;
>  
>  	ret = phy_pm_runtime_get_sync(phy);
> @@ -277,7 +277,7 @@ int phy_power_on(struct phy *phy)
>  {
>  	int ret = 0;
>  
> -	if (!phy)
> +	if (!phy || !phy->ops->power_on)
>  		goto out;
>  
>  	if (phy->pwr) {
> @@ -319,7 +319,7 @@ int phy_power_off(struct phy *phy)
>  {
>  	int ret;
>  
> -	if (!phy)
> +	if (!phy || !phy->ops->power_off)
>  		return 0;
>  
>  	mutex_lock(&phy->mutex);
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] phy: core: check whether ops callback function is assigned
  2016-12-27  8:31   ` Kishon Vijay Abraham I
@ 2016-12-27  8:37     ` Jaehoon Chung
  0 siblings, 0 replies; 3+ messages in thread
From: Jaehoon Chung @ 2016-12-27  8:37 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, linux-kernel

On 12/27/2016 05:31 PM, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Thursday 22 December 2016 03:12 PM, Jaehoon Chung wrote:
>> If some ops-> callback function are not assigend, then it should do the
>> unexpect behavior.
>> To prevent the potential NULL pointer dereference, check the each
>> callback functions before doing operation.
> 
> The call backs checks are done after the mutex. Moreover even if the call backs
> are not assigned, the user can call the phy ops for doing pm_runtime.

Yes. I found this patch also is wrong. Thanks for pointing out.

Best Regards,
Jaehoon Chung

> 
> Thanks
> Kishon
> 
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>  drivers/phy/phy-core.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> index a268f4d6f3e9..e4eb4431c8a4 100644
>> --- a/drivers/phy/phy-core.c
>> +++ b/drivers/phy/phy-core.c
>> @@ -219,7 +219,7 @@ int phy_init(struct phy *phy)
>>  {
>>  	int ret;
>>  
>> -	if (!phy)
>> +	if (!phy || !phy->ops->init)
>>  		return 0;
>>  
>>  	ret = phy_pm_runtime_get_sync(phy);
>> @@ -248,7 +248,7 @@ int phy_exit(struct phy *phy)
>>  {
>>  	int ret;
>>  
>> -	if (!phy)
>> +	if (!phy || !phy->ops->exit)
>>  		return 0;
>>  
>>  	ret = phy_pm_runtime_get_sync(phy);
>> @@ -277,7 +277,7 @@ int phy_power_on(struct phy *phy)
>>  {
>>  	int ret = 0;
>>  
>> -	if (!phy)
>> +	if (!phy || !phy->ops->power_on)
>>  		goto out;
>>  
>>  	if (phy->pwr) {
>> @@ -319,7 +319,7 @@ int phy_power_off(struct phy *phy)
>>  {
>>  	int ret;
>>  
>> -	if (!phy)
>> +	if (!phy || !phy->ops->power_off)
>>  		return 0;
>>  
>>  	mutex_lock(&phy->mutex);
>>
> 
> 
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-12-27  8:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20161222094224epcas1p49349afba1cf8468d350b66ca29e8590e@epcas1p4.samsung.com>
2016-12-22  9:42 ` [PATCH] phy: core: check whether ops callback function is assigned Jaehoon Chung
2016-12-27  8:31   ` Kishon Vijay Abraham I
2016-12-27  8:37     ` Jaehoon Chung

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).