public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
* re: ACPI / debugger: Add IO interface to access debugger functionalities
@ 2015-12-22 19:48 Dan Carpenter
  2015-12-24  5:45 ` Zheng, Lv
  2015-12-25  0:21 ` Zheng, Lv
  0 siblings, 2 replies; 3+ messages in thread
From: Dan Carpenter @ 2015-12-22 19:48 UTC (permalink / raw)
  To: lv.zheng; +Cc: linux-acpi

Hello Lv Zheng,

The patch 8cfb0cdf07e2: "ACPI / debugger: Add IO interface to access
debugger functionalities" from Dec 3, 2015, leads to the following
static checker warning:

	drivers/acpi/acpi_dbg.c:533 acpi_aml_open()
	error: double unlock 'mutex:&acpi_aml_io.lock'

drivers/acpi/acpi_dbg.c
   498          } else {
   499                  /*
   500                   * No writer is allowed unless the debugger thread is
   501                   * ready.
   502                   */
   503                  if (!(acpi_aml_io.flags & ACPI_AML_OPENED)) {
   504                          ret = -ENODEV;
   505                          goto err_lock;

Holding lock.

   506                  }
   507          }
   508          if (acpi_aml_active_reader == file) {
   509                  pr_debug("Opening debugger interface.\n");
   510                  mutex_unlock(&acpi_aml_io.lock);
   511  
   512                  pr_debug("Initializing debugger thread.\n");
   513                  status = acpi_initialize_debugger();
   514                  if (ACPI_FAILURE(status)) {
   515                          pr_err("Failed to initialize debugger.\n");
   516                          ret = -EINVAL;
   517                          goto err_lock;

Not holding lock.

   518                  }
   519                  acpi_aml_io.flags |= ACPI_AML_OPENED;
   520                  pr_debug("Debugger thread initialized.\n");
   521  
   522                  mutex_lock(&acpi_aml_io.lock);
   523                  acpi_aml_io.out_crc.head = acpi_aml_io.out_crc.tail = 0;
   524                  acpi_aml_io.in_crc.head = acpi_aml_io.in_crc.tail = 0;
   525                  pr_debug("Debugger interface opened.\n");
   526          }
   527          acpi_aml_io.users++;
   528  err_lock:
   529          if (IS_ERR_VALUE(ret)) {
   530                  if (acpi_aml_active_reader == file)
   531                          acpi_aml_active_reader = NULL;
   532          }
   533          mutex_unlock(&acpi_aml_io.lock);

Double onlock.

   534          return ret;
   535  }

regards,
dan carpenter

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

* RE: ACPI / debugger: Add IO interface to access debugger functionalities
  2015-12-22 19:48 ACPI / debugger: Add IO interface to access debugger functionalities Dan Carpenter
@ 2015-12-24  5:45 ` Zheng, Lv
  2015-12-25  0:21 ` Zheng, Lv
  1 sibling, 0 replies; 3+ messages in thread
From: Zheng, Lv @ 2015-12-24  5:45 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-acpi@vger.kernel.org

Hi,

Thanks for the report.


> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Dan Carpenter
> Sent: Wednesday, December 23, 2015 3:49 AM
> To: Zheng, Lv <lv.zheng@intel.com>
> Cc: linux-acpi@vger.kernel.org
> Subject: re: ACPI / debugger: Add IO interface to access debugger
> functionalities
> 
> Hello Lv Zheng,
> 
> The patch 8cfb0cdf07e2: "ACPI / debugger: Add IO interface to access
> debugger functionalities" from Dec 3, 2015, leads to the following
> static checker warning:
> 
> 	drivers/acpi/acpi_dbg.c:533 acpi_aml_open()
> 	error: double unlock 'mutex:&acpi_aml_io.lock'
> 
> drivers/acpi/acpi_dbg.c
>    498          } else {
>    499                  /*
>    500                   * No writer is allowed unless the debugger thread is
>    501                   * ready.
>    502                   */
>    503                  if (!(acpi_aml_io.flags & ACPI_AML_OPENED)) {
>    504                          ret = -ENODEV;
>    505                          goto err_lock;
> 
> Holding lock.
> 
>    506                  }
>    507          }
>    508          if (acpi_aml_active_reader == file) {
>    509                  pr_debug("Opening debugger interface.\n");
>    510                  mutex_unlock(&acpi_aml_io.lock);
>    511
>    512                  pr_debug("Initializing debugger thread.\n");
>    513                  status = acpi_initialize_debugger();
>    514                  if (ACPI_FAILURE(status)) {
>    515                          pr_err("Failed to initialize debugger.\n");
>    516                          ret = -EINVAL;
>    517                          goto err_lock;
> 
> Not holding lock.
> 
>    518                  }
>    520                  pr_debug("Debugger thread initialized.\n");
>    521
>    522                  mutex_lock(&acpi_aml_io.lock);
[Lv Zheng] 
Here the lock is held again.
I also noticed that the above line should be put under the locking environment:

>    519                  acpi_aml_io.flags |= ACPI_AML_OPENED;

Thanks for the report.
I'll submit a fix for this.

Best regards
-Lv

>    523                  acpi_aml_io.out_crc.head = acpi_aml_io.out_crc.tail = 0;
>    524                  acpi_aml_io.in_crc.head = acpi_aml_io.in_crc.tail = 0;
>    525                  pr_debug("Debugger interface opened.\n");
>    526          }
>    527          acpi_aml_io.users++;
>    528  err_lock:
>    529          if (IS_ERR_VALUE(ret)) {
>    530                  if (acpi_aml_active_reader == file)
>    531                          acpi_aml_active_reader = NULL;
>    532          }
>    533          mutex_unlock(&acpi_aml_io.lock);
> 
> Double onlock.
> 
>    534          return ret;
>    535  }
> 
> regards,
> dan carpenter
> --
> 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

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

* RE: ACPI / debugger: Add IO interface to access debugger functionalities
  2015-12-22 19:48 ACPI / debugger: Add IO interface to access debugger functionalities Dan Carpenter
  2015-12-24  5:45 ` Zheng, Lv
@ 2015-12-25  0:21 ` Zheng, Lv
  1 sibling, 0 replies; 3+ messages in thread
From: Zheng, Lv @ 2015-12-25  0:21 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-acpi@vger.kernel.org

Hi,

> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Dan Carpenter
> Subject: re: ACPI / debugger: Add IO interface to access debugger
> functionalities
> 
> Hello Lv Zheng,
> 
> The patch 8cfb0cdf07e2: "ACPI / debugger: Add IO interface to access
> debugger functionalities" from Dec 3, 2015, leads to the following
> static checker warning:
> 
> 	drivers/acpi/acpi_dbg.c:533 acpi_aml_open()
> 	error: double unlock 'mutex:&acpi_aml_io.lock'
> 
> drivers/acpi/acpi_dbg.c
>    498          } else {
>    499                  /*
>    500                   * No writer is allowed unless the debugger thread is
>    501                   * ready.
>    502                   */
>    503                  if (!(acpi_aml_io.flags & ACPI_AML_OPENED)) {
>    504                          ret = -ENODEV;
>    505                          goto err_lock;
> 
> Holding lock.
> 
>    506                  }
>    507          }
>    508          if (acpi_aml_active_reader == file) {
>    509                  pr_debug("Opening debugger interface.\n");
>    510                  mutex_unlock(&acpi_aml_io.lock);
>    511
>    512                  pr_debug("Initializing debugger thread.\n");
>    513                  status = acpi_initialize_debugger();
>    514                  if (ACPI_FAILURE(status)) {
>    515                          pr_err("Failed to initialize debugger.\n");
>    516                          ret = -EINVAL;
>    517                          goto err_lock;
> 
> Not holding lock.
[Lv Zheng] 
Oops.
This is indeed a problem.
I'll send another fix for this.

Thanks and best regards
-Lv

> 
>    518                  }
>    519                  acpi_aml_io.flags |= ACPI_AML_OPENED;
>    520                  pr_debug("Debugger thread initialized.\n");
>    521
>    522                  mutex_lock(&acpi_aml_io.lock);
>    523                  acpi_aml_io.out_crc.head = acpi_aml_io.out_crc.tail = 0;
>    524                  acpi_aml_io.in_crc.head = acpi_aml_io.in_crc.tail = 0;
>    525                  pr_debug("Debugger interface opened.\n");
>    526          }
>    527          acpi_aml_io.users++;
>    528  err_lock:
>    529          if (IS_ERR_VALUE(ret)) {
>    530                  if (acpi_aml_active_reader == file)
>    531                          acpi_aml_active_reader = NULL;
>    532          }
>    533          mutex_unlock(&acpi_aml_io.lock);
> 
> Double onlock.
> 
>    534          return ret;
>    535  }
> 
> regards,
> dan carpenter
> --
> 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

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

end of thread, other threads:[~2015-12-25  0:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-22 19:48 ACPI / debugger: Add IO interface to access debugger functionalities Dan Carpenter
2015-12-24  5:45 ` Zheng, Lv
2015-12-25  0:21 ` Zheng, Lv

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox