Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] ACPI: debug: don't allow debugging when ACPI is disabled
@ 2020-10-09 15:21 Jamie Iles
  2020-10-10  3:27 ` Hanjun Guo
  2020-10-12 13:04 ` [PATCHv2] " Jamie Iles
  0 siblings, 2 replies; 5+ messages in thread
From: Jamie Iles @ 2020-10-09 15:21 UTC (permalink / raw)
  To: linux-acpi; +Cc: Jamie Iles, Rafael J. Wysocki

If ACPI is disabled then loading the acpi_dbg module will result in the
following splat when lock debugging is enabled.

  DEBUG_LOCKS_WARN_ON(lock->magic != lock)
  WARNING: CPU: 0 PID: 1 at kernel/locking/mutex.c:938 __mutex_lock+0xa10/0x1290
  Kernel panic - not syncing: panic_on_warn set ...
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.9.0-rc8+ #103
  Hardware name: linux,dummy-virt (DT)
  Call trace:
   dump_backtrace+0x0/0x4d8
   show_stack+0x34/0x48
   dump_stack+0x174/0x1f8
   panic+0x360/0x7a0
   __warn+0x244/0x2ec
   report_bug+0x240/0x398
   bug_handler+0x50/0xc0
   call_break_hook+0x160/0x1d8
   brk_handler+0x30/0xc0
   do_debug_exception+0x184/0x340
   el1_dbg+0x48/0xb0
   el1_sync_handler+0x170/0x1c8
   el1_sync+0x80/0x100
   __mutex_lock+0xa10/0x1290
   mutex_lock_nested+0x6c/0xc0
   acpi_register_debugger+0x40/0x88
   acpi_aml_init+0xc4/0x114
   do_one_initcall+0x24c/0xb10
   kernel_init_freeable+0x690/0x728
   kernel_init+0x20/0x1e8
   ret_from_fork+0x10/0x18

Fail module loading to avoid this and any subsequent problems that might
arise by trying to debug AML when ACPI is disabled.

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Jamie Iles <jamie@nuviainc.com>
---
 drivers/acpi/acpi_dbg.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/acpi/acpi_dbg.c b/drivers/acpi/acpi_dbg.c
index 6041974c7627..fb7290338593 100644
--- a/drivers/acpi/acpi_dbg.c
+++ b/drivers/acpi/acpi_dbg.c
@@ -749,6 +749,9 @@ static int __init acpi_aml_init(void)
 {
 	int ret;
 
+	if (acpi_disabled)
+		return -ENODEV;
+
 	/* Initialize AML IO interface */
 	mutex_init(&acpi_aml_io.lock);
 	init_waitqueue_head(&acpi_aml_io.wait);
-- 
2.25.1


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

* Re: [PATCH] ACPI: debug: don't allow debugging when ACPI is disabled
  2020-10-09 15:21 [PATCH] ACPI: debug: don't allow debugging when ACPI is disabled Jamie Iles
@ 2020-10-10  3:27 ` Hanjun Guo
  2020-10-12 13:04 ` [PATCHv2] " Jamie Iles
  1 sibling, 0 replies; 5+ messages in thread
From: Hanjun Guo @ 2020-10-10  3:27 UTC (permalink / raw)
  To: Jamie Iles, linux-acpi; +Cc: Rafael J. Wysocki

On 2020/10/9 23:21, Jamie Iles wrote:
> If ACPI is disabled then loading the acpi_dbg module will result in the
> following splat when lock debugging is enabled.
> 
>    DEBUG_LOCKS_WARN_ON(lock->magic != lock)
>    WARNING: CPU: 0 PID: 1 at kernel/locking/mutex.c:938 __mutex_lock+0xa10/0x1290
>    Kernel panic - not syncing: panic_on_warn set ...
>    CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.9.0-rc8+ #103
>    Hardware name: linux,dummy-virt (DT)
>    Call trace:
>     dump_backtrace+0x0/0x4d8
>     show_stack+0x34/0x48
>     dump_stack+0x174/0x1f8
>     panic+0x360/0x7a0
>     __warn+0x244/0x2ec
>     report_bug+0x240/0x398
>     bug_handler+0x50/0xc0
>     call_break_hook+0x160/0x1d8
>     brk_handler+0x30/0xc0
>     do_debug_exception+0x184/0x340
>     el1_dbg+0x48/0xb0
>     el1_sync_handler+0x170/0x1c8
>     el1_sync+0x80/0x100
>     __mutex_lock+0xa10/0x1290
>     mutex_lock_nested+0x6c/0xc0
>     acpi_register_debugger+0x40/0x88
>     acpi_aml_init+0xc4/0x114
>     do_one_initcall+0x24c/0xb10
>     kernel_init_freeable+0x690/0x728
>     kernel_init+0x20/0x1e8
>     ret_from_fork+0x10/0x18

That's because the mutex acpi_debugger.lock will not be initialized
as acpi_debugger_init() is not called if ACPI is disabled.

If you add above commit log then make it easier to be understood.

> 
> Fail module loading to avoid this and any subsequent problems that might
> arise by trying to debug AML when ACPI is disabled.
> 

It's better to add a fix tag:

Fixes: 8cfb0cdf07e2 ("ACPI / debugger: Add IO interface to access 
debugger functionalities")

> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Signed-off-by: Jamie Iles <jamie@nuviainc.com>
> ---
>   drivers/acpi/acpi_dbg.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/acpi/acpi_dbg.c b/drivers/acpi/acpi_dbg.c
> index 6041974c7627..fb7290338593 100644
> --- a/drivers/acpi/acpi_dbg.c
> +++ b/drivers/acpi/acpi_dbg.c
> @@ -749,6 +749,9 @@ static int __init acpi_aml_init(void)
>   {
>   	int ret;
>   
> +	if (acpi_disabled)
> +		return -ENODEV;
> +
>   	/* Initialize AML IO interface */
>   	mutex_init(&acpi_aml_io.lock);
>   	init_waitqueue_head(&acpi_aml_io.wait);

With above comments addressed, feel free to add

Reviewed-by: Hanjun Guo <guohanjun@huawei.com>

Thanks
Hanjun

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

* [PATCHv2] ACPI: debug: don't allow debugging when ACPI is disabled
  2020-10-09 15:21 [PATCH] ACPI: debug: don't allow debugging when ACPI is disabled Jamie Iles
  2020-10-10  3:27 ` Hanjun Guo
@ 2020-10-12 13:04 ` Jamie Iles
  2020-10-14  0:43   ` Hanjun Guo
  2020-10-16 16:00   ` Rafael J. Wysocki
  1 sibling, 2 replies; 5+ messages in thread
From: Jamie Iles @ 2020-10-12 13:04 UTC (permalink / raw)
  To: linux-acpi; +Cc: Jamie Iles, Hanjun Guo, Rafael J. Wysocki

If ACPI is disabled then loading the acpi_dbg module will result in the
following splat when lock debugging is enabled.

  DEBUG_LOCKS_WARN_ON(lock->magic != lock)
  WARNING: CPU: 0 PID: 1 at kernel/locking/mutex.c:938 __mutex_lock+0xa10/0x1290
  Kernel panic - not syncing: panic_on_warn set ...
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.9.0-rc8+ #103
  Hardware name: linux,dummy-virt (DT)
  Call trace:
   dump_backtrace+0x0/0x4d8
   show_stack+0x34/0x48
   dump_stack+0x174/0x1f8
   panic+0x360/0x7a0
   __warn+0x244/0x2ec
   report_bug+0x240/0x398
   bug_handler+0x50/0xc0
   call_break_hook+0x160/0x1d8
   brk_handler+0x30/0xc0
   do_debug_exception+0x184/0x340
   el1_dbg+0x48/0xb0
   el1_sync_handler+0x170/0x1c8
   el1_sync+0x80/0x100
   __mutex_lock+0xa10/0x1290
   mutex_lock_nested+0x6c/0xc0
   acpi_register_debugger+0x40/0x88
   acpi_aml_init+0xc4/0x114
   do_one_initcall+0x24c/0xb10
   kernel_init_freeable+0x690/0x728
   kernel_init+0x20/0x1e8
   ret_from_fork+0x10/0x18

This is because acpi_debugger.lock has not been initialized as
acpi_debugger_init() is not called when ACPI is disabled.  Fail module
loading to avoid this and any subsequent problems that might arise by
trying to debug AML when ACPI is disabled.

Fixes: 8cfb0cdf07e2 ("ACPI / debugger: Add IO interface to access debugger functionalities")
Reviewed-by: Hanjun Guo <guohanjun@huawei.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Jamie Iles <jamie@nuviainc.com>
---
 drivers/acpi/acpi_dbg.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/acpi/acpi_dbg.c b/drivers/acpi/acpi_dbg.c
index 6041974c7627..fb7290338593 100644
--- a/drivers/acpi/acpi_dbg.c
+++ b/drivers/acpi/acpi_dbg.c
@@ -749,6 +749,9 @@ static int __init acpi_aml_init(void)
 {
 	int ret;
 
+	if (acpi_disabled)
+		return -ENODEV;
+
 	/* Initialize AML IO interface */
 	mutex_init(&acpi_aml_io.lock);
 	init_waitqueue_head(&acpi_aml_io.wait);
-- 
2.25.1


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

* Re: [PATCHv2] ACPI: debug: don't allow debugging when ACPI is disabled
  2020-10-12 13:04 ` [PATCHv2] " Jamie Iles
@ 2020-10-14  0:43   ` Hanjun Guo
  2020-10-16 16:00   ` Rafael J. Wysocki
  1 sibling, 0 replies; 5+ messages in thread
From: Hanjun Guo @ 2020-10-14  0:43 UTC (permalink / raw)
  To: Jamie Iles, linux-acpi; +Cc: Rafael J. Wysocki

On 2020/10/12 21:04, Jamie Iles wrote:
> If ACPI is disabled then loading the acpi_dbg module will result in the
> following splat when lock debugging is enabled.
> 
>    DEBUG_LOCKS_WARN_ON(lock->magic != lock)
>    WARNING: CPU: 0 PID: 1 at kernel/locking/mutex.c:938 __mutex_lock+0xa10/0x1290
>    Kernel panic - not syncing: panic_on_warn set ...
>    CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.9.0-rc8+ #103
>    Hardware name: linux,dummy-virt (DT)
>    Call trace:
>     dump_backtrace+0x0/0x4d8
>     show_stack+0x34/0x48
>     dump_stack+0x174/0x1f8
>     panic+0x360/0x7a0
>     __warn+0x244/0x2ec
>     report_bug+0x240/0x398
>     bug_handler+0x50/0xc0
>     call_break_hook+0x160/0x1d8
>     brk_handler+0x30/0xc0
>     do_debug_exception+0x184/0x340
>     el1_dbg+0x48/0xb0
>     el1_sync_handler+0x170/0x1c8
>     el1_sync+0x80/0x100
>     __mutex_lock+0xa10/0x1290
>     mutex_lock_nested+0x6c/0xc0
>     acpi_register_debugger+0x40/0x88
>     acpi_aml_init+0xc4/0x114
>     do_one_initcall+0x24c/0xb10
>     kernel_init_freeable+0x690/0x728
>     kernel_init+0x20/0x1e8
>     ret_from_fork+0x10/0x18
> 
> This is because acpi_debugger.lock has not been initialized as
> acpi_debugger_init() is not called when ACPI is disabled.  Fail module
> loading to avoid this and any subsequent problems that might arise by
> trying to debug AML when ACPI is disabled.
> 
> Fixes: 8cfb0cdf07e2 ("ACPI / debugger: Add IO interface to access debugger functionalities")
> Reviewed-by: Hanjun Guo <guohanjun@huawei.com>

My review still valid, thanks!

Hanjun

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

* Re: [PATCHv2] ACPI: debug: don't allow debugging when ACPI is disabled
  2020-10-12 13:04 ` [PATCHv2] " Jamie Iles
  2020-10-14  0:43   ` Hanjun Guo
@ 2020-10-16 16:00   ` Rafael J. Wysocki
  1 sibling, 0 replies; 5+ messages in thread
From: Rafael J. Wysocki @ 2020-10-16 16:00 UTC (permalink / raw)
  To: Jamie Iles; +Cc: ACPI Devel Maling List, Hanjun Guo, Rafael J. Wysocki

On Mon, Oct 12, 2020 at 3:05 PM Jamie Iles <jamie@nuviainc.com> wrote:
>
> If ACPI is disabled then loading the acpi_dbg module will result in the
> following splat when lock debugging is enabled.
>
>   DEBUG_LOCKS_WARN_ON(lock->magic != lock)
>   WARNING: CPU: 0 PID: 1 at kernel/locking/mutex.c:938 __mutex_lock+0xa10/0x1290
>   Kernel panic - not syncing: panic_on_warn set ...
>   CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.9.0-rc8+ #103
>   Hardware name: linux,dummy-virt (DT)
>   Call trace:
>    dump_backtrace+0x0/0x4d8
>    show_stack+0x34/0x48
>    dump_stack+0x174/0x1f8
>    panic+0x360/0x7a0
>    __warn+0x244/0x2ec
>    report_bug+0x240/0x398
>    bug_handler+0x50/0xc0
>    call_break_hook+0x160/0x1d8
>    brk_handler+0x30/0xc0
>    do_debug_exception+0x184/0x340
>    el1_dbg+0x48/0xb0
>    el1_sync_handler+0x170/0x1c8
>    el1_sync+0x80/0x100
>    __mutex_lock+0xa10/0x1290
>    mutex_lock_nested+0x6c/0xc0
>    acpi_register_debugger+0x40/0x88
>    acpi_aml_init+0xc4/0x114
>    do_one_initcall+0x24c/0xb10
>    kernel_init_freeable+0x690/0x728
>    kernel_init+0x20/0x1e8
>    ret_from_fork+0x10/0x18
>
> This is because acpi_debugger.lock has not been initialized as
> acpi_debugger_init() is not called when ACPI is disabled.  Fail module
> loading to avoid this and any subsequent problems that might arise by
> trying to debug AML when ACPI is disabled.
>
> Fixes: 8cfb0cdf07e2 ("ACPI / debugger: Add IO interface to access debugger functionalities")
> Reviewed-by: Hanjun Guo <guohanjun@huawei.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Signed-off-by: Jamie Iles <jamie@nuviainc.com>
> ---
>  drivers/acpi/acpi_dbg.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/acpi/acpi_dbg.c b/drivers/acpi/acpi_dbg.c
> index 6041974c7627..fb7290338593 100644
> --- a/drivers/acpi/acpi_dbg.c
> +++ b/drivers/acpi/acpi_dbg.c
> @@ -749,6 +749,9 @@ static int __init acpi_aml_init(void)
>  {
>         int ret;
>
> +       if (acpi_disabled)
> +               return -ENODEV;
> +
>         /* Initialize AML IO interface */
>         mutex_init(&acpi_aml_io.lock);
>         init_waitqueue_head(&acpi_aml_io.wait);
> --

Applied as 5.10-rc material, thanks!

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-09 15:21 [PATCH] ACPI: debug: don't allow debugging when ACPI is disabled Jamie Iles
2020-10-10  3:27 ` Hanjun Guo
2020-10-12 13:04 ` [PATCHv2] " Jamie Iles
2020-10-14  0:43   ` Hanjun Guo
2020-10-16 16:00   ` Rafael J. Wysocki

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org
	public-inbox-index linux-acpi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git