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 }
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
Hi,
Thanks for the report.
Quoted 43 lines
-----Original Message-----
From: [email protected] [mailto:linux-acpi- [email protected]] On Behalf Of Dan Carpenter
Sent: Wednesday, December 23, 2015 3:49 AM
To: Zheng, Lv <[email protected]>
Cc: [email protected]
Subject: re: ACPI / debugger: Add IO interface to access debugger functionalitiesHello 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.c498 } 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;
Quoted 6 lines
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:
Quoted 1 line
519 acpi_aml_io.flags |= ACPI_AML_OPENED;
Thanks for the report.
I'll submit a fix for this.
Best regards
-Lv
Quoted 23 lines
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi,
Quoted 39 lines
From: [email protected] [mailto:linux-acpi- [email protected]] On Behalf Of Dan Carpenter
Subject: re: ACPI / debugger: Add IO interface to access debugger functionalitiesHello 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.c498 } 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;
Quoted 1 line
Not holding lock.
[Lv Zheng]
Oops.
This is indeed a problem.
I'll send another fix for this.
Thanks and best regards
-Lv
Quoted 29 lines
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html