[PATCH] Thermal: Fix synchronization issues in thermal_sys.c

[PATCH] Thermal: Fix synchronization issues in thermal_sys.c

[Durgadoss R]

This patch fixes the following mutex and NULL pointer
problems in thermal_sys.c:
 * mutex_unlock fix in update_temperature function
 * mutex_unlock/NULL check fix in bind_cdev function
 * NULL check fix in bind_tz function

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/thermal/thermal_sys.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 4f77d89..848553d 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -252,8 +252,8 @@ static void bind_cdev(struct thermal_cooling_device *cdev)
 		}
 
 		tzp = pos->tzp;
-		if (!tzp->tbp)
-			return;
+		if (!tzp || !tzp->tbp)
+			continue;
 
 		for (i = 0; i < tzp->num_tbps; i++) {
 			if (tzp->tbp[i].cdev || !tzp->tbp[i].match)
@@ -289,7 +289,7 @@ static void bind_tz(struct thermal_zone_device *tz)
 		goto exit;
 	}
 
-	if (!tzp->tbp)
+	if (!tzp || !tzp->tbp)
 		goto exit;
 
 	list_for_each_entry(pos, &thermal_cdev_list, node) {
@@ -390,12 +390,13 @@ static void update_temperature(struct thermal_zone_device *tz)
 	ret = tz->ops->get_temp(tz, &temp);
 	if (ret) {
 		pr_warn("failed to read out thermal zone %d\n", tz->id);
-		return;
+		goto exit;
 	}
 
 	tz->last_temperature = tz->temperature;
 	tz->temperature = temp;
 
+exit:
 	mutex_unlock(&tz->lock);
 }
 
-- 
1.7.9.5

Re: [PATCH] Thermal: Fix synchronization issues in thermal_sys.c

[Zhang Rui]

On 四, 2012-09-27 at 11:38 +0530, Durgadoss R wrote:
> This patch fixes the following mutex and NULL pointer
> problems in thermal_sys.c:
>  * mutex_unlock fix in update_temperature function
>  * mutex_unlock/NULL check fix in bind_cdev function
>  * NULL check fix in bind_tz function
> 
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
>  drivers/thermal/thermal_sys.c |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index 4f77d89..848553d 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -252,8 +252,8 @@ static void bind_cdev(struct thermal_cooling_device *cdev)
>  		}
>  
>  		tzp = pos->tzp;
> -		if (!tzp->tbp)
> -			return;
> +		if (!tzp || !tzp->tbp)
> +			continue;
>  
>  		for (i = 0; i < tzp->num_tbps; i++) {
>  			if (tzp->tbp[i].cdev || !tzp->tbp[i].match)
> @@ -289,7 +289,7 @@ static void bind_tz(struct thermal_zone_device *tz)
>  		goto exit;
>  	}
>  
> -	if (!tzp->tbp)
> +	if (!tzp || !tzp->tbp)
>  		goto exit;
>  
actually, this is not a problem.
I checked the code just now, tzp can not be NULL if the code runs here.

>  	list_for_each_entry(pos, &thermal_cdev_list, node) {
> @@ -390,12 +390,13 @@ static void update_temperature(struct thermal_zone_device *tz)
>  	ret = tz->ops->get_temp(tz, &temp);
>  	if (ret) {
>  		pr_warn("failed to read out thermal zone %d\n", tz->id);
> -		return;
> +		goto exit;
>  	}
>  
>  	tz->last_temperature = tz->temperature;
>  	tz->temperature = temp;
>  
> +exit:
>  	mutex_unlock(&tz->lock);
>  }
>  


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] Thermal: Fix synchronization issues in thermal_sys.c

[R, Durgadoss]

Hi Rui,

> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Zhang Rui
> Sent: Thursday, September 27, 2012 11:45 AM
> To: R, Durgadoss
> Cc: lenb@kernel.org; linux-acpi@vger.kernel.org;
> dan.carpenter@oracle.com; hughd@google.com; linux-
> next@vger.kernel.org
> Subject: Re: [PATCH] Thermal: Fix synchronization issues in thermal_sys.c
> 
> On 四, 2012-09-27 at 11:38 +0530, Durgadoss R wrote:
> > This patch fixes the following mutex and NULL pointer
> > problems in thermal_sys.c:
> >  * mutex_unlock fix in update_temperature function
> >  * mutex_unlock/NULL check fix in bind_cdev function
> >  * NULL check fix in bind_tz function
> >
> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > ---
> >  drivers/thermal/thermal_sys.c |    9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> > index 4f77d89..848553d 100644
> > --- a/drivers/thermal/thermal_sys.c
> > +++ b/drivers/thermal/thermal_sys.c
> > @@ -252,8 +252,8 @@ static void bind_cdev(struct thermal_cooling_device
> *cdev)
> >  		}
> >
> >  		tzp = pos->tzp;
> > -		if (!tzp->tbp)
> > -			return;
> > +		if (!tzp || !tzp->tbp)
> > +			continue;
> >
> >  		for (i = 0; i < tzp->num_tbps; i++) {
> >  			if (tzp->tbp[i].cdev || !tzp->tbp[i].match)
> > @@ -289,7 +289,7 @@ static void bind_tz(struct thermal_zone_device *tz)
> >  		goto exit;
> >  	}
> >
> > -	if (!tzp->tbp)
> > +	if (!tzp || !tzp->tbp)
> >  		goto exit;
> >
> actually, this is not a problem.
> I checked the code just now, tzp can not be NULL if the code runs here.

I agree, I saw a similar kind of change needed in bind_tz, and hence
added it here. Should we carry it as such ? or you want me to refresh by
changing it ?

Thanks for looking into this quickly.

Thanks,
Durga

Re: [PATCH] Thermal: Fix synchronization issues in thermal_sys.c

[Sedat Dilek]

On Thu, Sep 27, 2012 at 8:21 AM, R, Durgadoss <durgadoss.r@intel.com> wrote:
> Hi Rui,
>
>> -----Original Message-----
>> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
>> owner@vger.kernel.org] On Behalf Of Zhang Rui
>> Sent: Thursday, September 27, 2012 11:45 AM
>> To: R, Durgadoss
>> Cc: lenb@kernel.org; linux-acpi@vger.kernel.org;
>> dan.carpenter@oracle.com; hughd@google.com; linux-
>> next@vger.kernel.org
>> Subject: Re: [PATCH] Thermal: Fix synchronization issues in thermal_sys.c
>>
>> On 四, 2012-09-27 at 11:38 +0530, Durgadoss R wrote:
>> > This patch fixes the following mutex and NULL pointer
>> > problems in thermal_sys.c:
>> >  * mutex_unlock fix in update_temperature function
>> >  * mutex_unlock/NULL check fix in bind_cdev function
>> >  * NULL check fix in bind_tz function
>> >
>> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
>> > ---
>> >  drivers/thermal/thermal_sys.c |    9 +++++----
>> >  1 file changed, 5 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
>> > index 4f77d89..848553d 100644
>> > --- a/drivers/thermal/thermal_sys.c
>> > +++ b/drivers/thermal/thermal_sys.c
>> > @@ -252,8 +252,8 @@ static void bind_cdev(struct thermal_cooling_device
>> *cdev)
>> >             }
>> >
>> >             tzp = pos->tzp;
>> > -           if (!tzp->tbp)
>> > -                   return;
>> > +           if (!tzp || !tzp->tbp)
>> > +                   continue;
>> >
>> >             for (i = 0; i < tzp->num_tbps; i++) {
>> >                     if (tzp->tbp[i].cdev || !tzp->tbp[i].match)
>> > @@ -289,7 +289,7 @@ static void bind_tz(struct thermal_zone_device *tz)
>> >             goto exit;
>> >     }
>> >
>> > -   if (!tzp->tbp)
>> > +   if (!tzp || !tzp->tbp)
>> >             goto exit;
>> >
>> actually, this is not a problem.
>> I checked the code just now, tzp can not be NULL if the code runs here.
>
> I agree, I saw a similar kind of change needed in bind_tz, and hence
> added it here. Should we carry it as such ? or you want me to refresh by
> changing it ?
>
> Thanks for looking into this quickly.
>

Looks like you will sent a v2 of this patch.
Feel free to add a...

        Reported-by: Sedat Dilek <sedat.dilek@gmail.com>

,,, and don't forget Hugh :-).

- Sedat -

> Thanks,
> Durga
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Thermal: Fix synchronization issues in thermal_sys.c

[Dan Carpenter]

On Thu, Sep 27, 2012 at 08:25:11AM +0200, Sedat Dilek wrote:
> Looks like you will sent a v2 of this patch.
> Feel free to add a...
> 
>         Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
> 
> ,,, and don't forget Hugh :-).

And me!  I want a reported by cookie too!

regards,
dan carpenter