netdev - Re: [PATCH 1/1] Bluetooth: fix race condition in hidp_session

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <CABBYNZL2RPLFfjs=EaS4khqLSXjwN2d=1FqY3Z-feX-j_QmOnw@mail.gmail.com>
Date:   Fri, 3 Mar 2023 10:08:27 -0800
From:   Luiz Augusto von Dentz <luiz.dentz@...il.com>
To:     lm0963 <lm0963hack@...il.com>
Cc:     Marcel Holtmann <marcel@...tmann.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, security@...nel.org,
        linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, Tedd Ho-Jeong An <hj.tedd.an@...il.com>
Subject: Re: [PATCH 1/1] Bluetooth: fix race condition in hidp_session_thread

Hi Tedd,

On Wed, Mar 1, 2023 at 10:18 PM lm0963 <lm0963hack@...il.com> wrote:
>
> There is a potential race condition in hidp_session_thread that may
> lead to use-after-free. For instance, the timer is active while
> hidp_del_timer is called in hidp_session_thread(). After hidp_session_put,
> then 'session' will be freed, causing kernel panic when hidp_idle_timeout
> is running.
>
> The solution is to use del_timer_sync instead of del_timer.
>
> Here is the call trace:
>
> ? hidp_session_probe+0x780/0x780
> call_timer_fn+0x2d/0x1e0
> __run_timers.part.0+0x569/0x940
> hidp_session_probe+0x780/0x780
> call_timer_fn+0x1e0/0x1e0
> ktime_get+0x5c/0xf0
> lapic_next_deadline+0x2c/0x40
> clockevents_program_event+0x205/0x320
> run_timer_softirq+0xa9/0x1b0
> __do_softirq+0x1b9/0x641
> __irq_exit_rcu+0xdc/0x190
> irq_exit_rcu+0xe/0x20
> sysvec_apic_timer_interrupt+0xa1/0xc0
>
> Cc: stable@...r.kernel.org
> Signed-off-by: Min Li <lm0963hack@...il.com>
> ---
>  net/bluetooth/hidp/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index bed1a7b9205c..707f229f896a 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -433,7 +433,7 @@ static void hidp_set_timer(struct hidp_session *session)
>  static void hidp_del_timer(struct hidp_session *session)
>  {
>         if (session->idle_to > 0)
> -               del_timer(&session->timer);
> +               del_timer_sync(&session->timer);
>  }
>
>  static void hidp_process_report(struct hidp_session *session, int type,
> --
> 2.25.1

Looks like CI didn't pick up this one.

-- 
Luiz Augusto von Dentz