MIME-Version: 1.0
References: <CAAgLYK7pm06588j+W7F0+2mgfVs1Sr7ioL4x+Bd-TZfV-Zw9Pg@mail.gmail.com>
In-Reply-To: <CAAgLYK7pm06588j+W7F0+2mgfVs1Sr7ioL4x+Bd-TZfV-Zw9Pg@mail.gmail.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Fri, 3 Mar 2023 10:08:27 -0800
Message-ID: <CABBYNZL2RPLFfjs=EaS4khqLSXjwN2d=1FqY3Z-feX-j_QmOnw@mail.gmail.com>
Subject: Re: [PATCH 1/1] Bluetooth: fix race condition in hidp_session_thread
To:     lm0963 <lm0963hack@gmail.com>
Cc:     Marcel Holtmann <marcel@holtmann.org>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, security@kernel.org,
        linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org, Tedd Ho-Jeong An <hj.tedd.an@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Tedd,

On Wed, Mar 1, 2023 at 10:18=E2=80=AFPM lm0963 <lm0963hack@gmail.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@vger.kernel.org
> Signed-off-by: Min Li <lm0963hack@gmail.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 *sessi=
on)
>  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.

--=20
Luiz Augusto von Dentz