[PATCH] Bluetooth: btnxpuart: Shutdown timer and prevent rearming when driver unloading

[PATCH] Bluetooth: btnxpuart: Shutdown timer and prevent rearming when driver unloading

[ziniu.wang_1]

From: Luke Wang <ziniu.wang_1@nxp.com>

When unload the btnxpuart driver, its associated timer will be deleted.
If the timer happens to be modified at this moment, it leads to the
kernel call this timer even after the driver unloaded, resulting in
kernel panic.
Use timer_shutdown_sync() instead of del_timer_sync() to prevent rearming.

panic log:
  Internal error: Oops: 0000000086000007 [#1] PREEMPT SMP
  Modules linked in: algif_hash algif_skcipher af_alg moal(O) mlan(O) crct10dif_ce polyval_ce polyval_generic   snd_soc_imx_card snd_soc_fsl_asoc_card snd_soc_imx_audmux mxc_jpeg_encdec v4l2_jpeg snd_soc_wm8962 snd_soc_fsl_micfil   snd_soc_fsl_sai flexcan snd_soc_fsl_utils ap130x rpmsg_ctrl imx_pcm_dma can_dev rpmsg_char pwm_fan fuse [last unloaded:   btnxpuart]
  CPU: 5 PID: 723 Comm: memtester Tainted: G           O       6.6.23-lts-next-06207-g4aef2658ac28 #1
  Hardware name: NXP i.MX95 19X19 board (DT)
  pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  pc : 0xffff80007a2cf464
  lr : call_timer_fn.isra.0+0x24/0x80
...
  Call trace:
   0xffff80007a2cf464
   __run_timers+0x234/0x280
   run_timer_softirq+0x20/0x40
   __do_softirq+0x100/0x26c
   ____do_softirq+0x10/0x1c
   call_on_irq_stack+0x24/0x4c
   do_softirq_own_stack+0x1c/0x2c
   irq_exit_rcu+0xc0/0xdc
   el0_interrupt+0x54/0xd8
   __el0_irq_handler_common+0x18/0x24
   el0t_64_irq_handler+0x10/0x1c
   el0t_64_irq+0x190/0x194
  Code: ???????? ???????? ???????? ???????? (????????)
  ---[ end trace 0000000000000000 ]---
  Kernel panic - not syncing: Oops: Fatal exception in interrupt
  SMP: stopping secondary CPUs
  Kernel Offset: disabled
  CPU features: 0x0,c0000000,40028143,1000721b
  Memory Limit: none
  ---[ end Kernel panic - not syncing: Oops: Fatal exception in interrupt ]---

Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/bluetooth/btnxpuart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index 7f88b6f52f26..77f974a5251b 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -328,7 +328,7 @@ static void ps_cancel_timer(struct btnxpuart_dev *nxpdev)
 	struct ps_data *psdata = &nxpdev->psdata;
 
 	flush_work(&psdata->work);
-	del_timer_sync(&psdata->ps_timer);
+	timer_shutdown_sync(&psdata->ps_timer);
 }
 
 static void ps_control(struct hci_dev *hdev, u8 ps_state)
-- 
2.34.1

RE: Bluetooth: btnxpuart: Shutdown timer and prevent rearming when driver unloading

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 4457 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=853957

---Test result---

Test Summary:
CheckPatch                    FAIL      0.90 seconds
GitLint                       FAIL      0.54 seconds
SubjectPrefix                 PASS      0.11 seconds
BuildKernel                   PASS      30.40 seconds
CheckAllWarning               PASS      33.07 seconds
CheckSparse                   PASS      38.99 seconds
CheckSmatch                   FAIL      35.14 seconds
BuildKernel32                 PASS      28.50 seconds
TestRunnerSetup               PASS      518.25 seconds
TestRunner_l2cap-tester       PASS      18.12 seconds
TestRunner_iso-tester         PASS      28.05 seconds
TestRunner_bnep-tester        PASS      4.73 seconds
TestRunner_mgmt-tester        PASS      109.20 seconds
TestRunner_rfcomm-tester      PASS      7.29 seconds
TestRunner_sco-tester         PASS      15.88 seconds
TestRunner_ioctl-tester       PASS      7.66 seconds
TestRunner_mesh-tester        PASS      9.11 seconds
TestRunner_smp-tester         PASS      6.80 seconds
TestRunner_userchan-tester    PASS      4.87 seconds
IncrementalBuild              PASS      27.76 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: btnxpuart: Shutdown timer and prevent rearming when driver unloading
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#125: 
  Modules linked in: algif_hash algif_skcipher af_alg moal(O) mlan(O) crct10dif_ce polyval_ce polyval_generic   snd_soc_imx_card snd_soc_fsl_asoc_card snd_soc_imx_audmux mxc_jpeg_encdec v4l2_jpeg snd_soc_wm8962 snd_soc_fsl_micfil   snd_soc_fsl_sai flexcan snd_soc_fsl_utils ap130x rpmsg_ctrl imx_pcm_dma can_dev rpmsg_char pwm_fan fuse [last unloaded:   btnxpuart]

total: 0 errors, 1 warnings, 8 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/src/13666877.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: btnxpuart: Shutdown timer and prevent rearming when driver unloading

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
13: B1 Line exceeds max length (364>80): "  Modules linked in: algif_hash algif_skcipher af_alg moal(O) mlan(O) crct10dif_ce polyval_ce polyval_generic   snd_soc_imx_card snd_soc_fsl_asoc_card snd_soc_imx_audmux mxc_jpeg_encdec v4l2_jpeg snd_soc_wm8962 snd_soc_fsl_micfil   snd_soc_fsl_sai flexcan snd_soc_fsl_utils ap130x rpmsg_ctrl imx_pcm_dma can_dev rpmsg_char pwm_fan fuse [last unloaded:   btnxpuart]"
14: B1 Line exceeds max length (101>80): "  CPU: 5 PID: 723 Comm: memtester Tainted: G           O       6.6.23-lts-next-06207-g4aef2658ac28 #1"
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:

Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: net/bluetooth/hci_core.o] Error 139
make[4]: *** Deleting file 'net/bluetooth/hci_core.o'
make[3]: *** [scripts/Makefile.build:485: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: net] Error 2
make[2]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bcm203x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bcm203x.o'
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [scripts/Makefile.build:485: drivers/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: drivers] Error 2
make[1]: *** [/github/workspace/src/src/Makefile:1919: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2


---
Regards,
Linux Bluetooth

Re: [PATCH] Bluetooth: btnxpuart: Shutdown timer and prevent rearming when driver unloading

[patchwork-bot+bluetooth]

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Fri, 17 May 2024 19:15:35 +0800 you wrote:
> From: Luke Wang <ziniu.wang_1@nxp.com>
> 
> When unload the btnxpuart driver, its associated timer will be deleted.
> If the timer happens to be modified at this moment, it leads to the
> kernel call this timer even after the driver unloaded, resulting in
> kernel panic.
> Use timer_shutdown_sync() instead of del_timer_sync() to prevent rearming.
> 
> [...]

Here is the summary with links:
  - Bluetooth: btnxpuart: Shutdown timer and prevent rearming when driver unloading
    https://git.kernel.org/bluetooth/bluetooth-next/c/3afc41cbec84

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

RE: [PATCH] Bluetooth: btnxpuart: Shutdown timer and prevent rearming when driver unloading

[Bough Chen]

> -----Original Message-----
> From: Luke Wang <ziniu.wang_1@nxp.com>
> Sent: 2024年5月17日 19:16
> To: marcel@holtmann.org; luiz.dentz@gmail.com
> Cc: linux-bluetooth@vger.kernel.org; linux-kernel@vger.kernel.org; Amitkumar
> Karwar <amitkumar.karwar@nxp.com>; Rohit Fule <rohit.fule@nxp.com>;
> Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>; Sherry Sun
> <sherry.sun@nxp.com>; Bough Chen <haibo.chen@nxp.com>; Jun Li
> <jun.li@nxp.com>; Guillaume Legoupil <guillaume.legoupil@nxp.com>; Salim
> Chebbo <salim.chebbo@nxp.com>; imx@lists.linux.dev
> Subject: [PATCH] Bluetooth: btnxpuart: Shutdown timer and prevent rearming
> when driver unloading
> 
> From: Luke Wang <ziniu.wang_1@nxp.com>
> 
> When unload the btnxpuart driver, its associated timer will be deleted.
> If the timer happens to be modified at this moment, it leads to the kernel call
> this timer even after the driver unloaded, resulting in kernel panic.
> Use timer_shutdown_sync() instead of del_timer_sync() to prevent rearming.
> 
> panic log:
>   Internal error: Oops: 0000000086000007 [#1] PREEMPT SMP
>   Modules linked in: algif_hash algif_skcipher af_alg moal(O) mlan(O)
> crct10dif_ce polyval_ce polyval_generic   snd_soc_imx_card
> snd_soc_fsl_asoc_card snd_soc_imx_audmux mxc_jpeg_encdec v4l2_jpeg
> snd_soc_wm8962 snd_soc_fsl_micfil   snd_soc_fsl_sai flexcan snd_soc_fsl_utils
> ap130x rpmsg_ctrl imx_pcm_dma can_dev rpmsg_char pwm_fan fuse [last
> unloaded:   btnxpuart]
>   CPU: 5 PID: 723 Comm: memtester Tainted: G           O
> 6.6.23-lts-next-06207-g4aef2658ac28 #1
>   Hardware name: NXP i.MX95 19X19 board (DT)
>   pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>   pc : 0xffff80007a2cf464
>   lr : call_timer_fn.isra.0+0x24/0x80
> ...
>   Call trace:
>    0xffff80007a2cf464
>    __run_timers+0x234/0x280
>    run_timer_softirq+0x20/0x40
>    __do_softirq+0x100/0x26c
>    ____do_softirq+0x10/0x1c
>    call_on_irq_stack+0x24/0x4c
>    do_softirq_own_stack+0x1c/0x2c
>    irq_exit_rcu+0xc0/0xdc
>    el0_interrupt+0x54/0xd8
>    __el0_irq_handler_common+0x18/0x24
>    el0t_64_irq_handler+0x10/0x1c
>    el0t_64_irq+0x190/0x194
>   Code: ???????? ???????? ???????? ???????? (????????)
>   ---[ end trace 0000000000000000 ]---
>   Kernel panic - not syncing: Oops: Fatal exception in interrupt
>   SMP: stopping secondary CPUs
>   Kernel Offset: disabled
>   CPU features: 0x0,c0000000,40028143,1000721b
>   Memory Limit: none
>   ---[ end Kernel panic - not syncing: Oops: Fatal exception in interrupt ]---


Hi all,

This patch is already accepted, but I notice it lack fix tag, can anyone help add the following fix tag?
Fixes: 689ca16e5232 ("Bluetooth: NXP: Add protocol support for NXP Bluetooth chipsets")
Cc: stable@vger.kernel.org

This patch should also put into stable tree. I add the stable tree mail list here. 

Best Regards
Haibo Chen
> 
> Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
> ---
>  drivers/bluetooth/btnxpuart.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c index
> 7f88b6f52f26..77f974a5251b 100644
> --- a/drivers/bluetooth/btnxpuart.c
> +++ b/drivers/bluetooth/btnxpuart.c
> @@ -328,7 +328,7 @@ static void ps_cancel_timer(struct btnxpuart_dev
> *nxpdev)
>  	struct ps_data *psdata = &nxpdev->psdata;
> 
>  	flush_work(&psdata->work);
> -	del_timer_sync(&psdata->ps_timer);
> +	timer_shutdown_sync(&psdata->ps_timer);
>  }
> 
>  static void ps_control(struct hci_dev *hdev, u8 ps_state)
> --
> 2.34.1