* [PATCH v2] Bluetooth: hci_core: Fix poential Use-after-Free bug in hci_remove_adv_monitor
@ 2023-02-17 10:02 Zheng Wang
2023-02-17 10:34 ` [v2] " bluez.test.bot
2023-03-13 9:55 ` [PATCH v2] " Zheng Hacker
0 siblings, 2 replies; 6+ messages in thread
From: Zheng Wang @ 2023-02-17 10:02 UTC (permalink / raw)
To: marcel
Cc: hackerzheng666, alex000young, johan.hedberg, luiz.dentz, davem,
edumazet, kuba, pabeni, linux-bluetooth, netdev, linux-kernel,
pmenzel, Zheng Wang
In hci_remove_adv_monitor, if it gets into HCI_ADV_MONITOR_EXT_MSFT case,
the function will free the monitor and print its handle after that.
Fix it by removing the logging into msft_le_cancel_monitor_advertisement_cb
before calling hci_free_adv_monitor.
Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
---
v2:
- move the logging inside msft_remove_monitor suggested by Luiz
---
net/bluetooth/hci_core.c | 2 --
net/bluetooth/msft.c | 2 ++
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index b65c3aabcd53..69b82c2907ff 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1981,8 +1981,6 @@ static int hci_remove_adv_monitor(struct hci_dev *hdev,
case HCI_ADV_MONITOR_EXT_MSFT:
status = msft_remove_monitor(hdev, monitor);
- bt_dev_dbg(hdev, "%s remove monitor %d msft status %d",
- hdev->name, monitor->handle, status);
break;
}
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index bee6a4c656be..4b35f0ed1360 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -286,6 +286,8 @@ static int msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
* suspend. It will be re-monitored on resume.
*/
if (!msft->suspending) {
+ bt_dev_dbg(hdev, "%s remove monitor %d status %d", hdev->name,
+ monitor->handle, status);
hci_free_adv_monitor(hdev, monitor);
/* Clear any monitored devices by this Adv Monitor */
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [v2] Bluetooth: hci_core: Fix poential Use-after-Free bug in hci_remove_adv_monitor
2023-02-17 10:02 [PATCH v2] Bluetooth: hci_core: Fix poential Use-after-Free bug in hci_remove_adv_monitor Zheng Wang
@ 2023-02-17 10:34 ` bluez.test.bot
2023-03-13 9:55 ` [PATCH v2] " Zheng Hacker
1 sibling, 0 replies; 6+ messages in thread
From: bluez.test.bot @ 2023-02-17 10:34 UTC (permalink / raw)
To: linux-bluetooth, zyytlz.wz
[-- Attachment #1: Type: text/plain, Size: 2077 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=722888
---Test result---
Test Summary:
CheckPatch PASS 0.80 seconds
GitLint FAIL 0.66 seconds
SubjectPrefix PASS 0.12 seconds
BuildKernel PASS 30.50 seconds
CheckAllWarning PASS 33.46 seconds
CheckSparse PASS 37.77 seconds
CheckSmatch PASS 107.02 seconds
BuildKernel32 PASS 29.51 seconds
TestRunnerSetup PASS 427.93 seconds
TestRunner_l2cap-tester PASS 16.08 seconds
TestRunner_iso-tester PASS 16.20 seconds
TestRunner_bnep-tester PASS 5.32 seconds
TestRunner_mgmt-tester PASS 106.40 seconds
TestRunner_rfcomm-tester PASS 8.53 seconds
TestRunner_sco-tester PASS 7.86 seconds
TestRunner_ioctl-tester PASS 9.20 seconds
TestRunner_mesh-tester PASS 6.70 seconds
TestRunner_smp-tester PASS 7.74 seconds
TestRunner_userchan-tester PASS 5.63 seconds
IncrementalBuild PASS 27.62 seconds
Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v2] Bluetooth: hci_core: Fix poential Use-after-Free bug in hci_remove_adv_monitor
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
1: T1 Title exceeds max length (83>80): "[v2] Bluetooth: hci_core: Fix poential Use-after-Free bug in hci_remove_adv_monitor"
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Bluetooth: hci_core: Fix poential Use-after-Free bug in hci_remove_adv_monitor
2023-02-17 10:02 [PATCH v2] Bluetooth: hci_core: Fix poential Use-after-Free bug in hci_remove_adv_monitor Zheng Wang
2023-02-17 10:34 ` [v2] " bluez.test.bot
@ 2023-03-13 9:55 ` Zheng Hacker
2023-03-14 15:06 ` Simon Horman
1 sibling, 1 reply; 6+ messages in thread
From: Zheng Hacker @ 2023-03-13 9:55 UTC (permalink / raw)
To: Zheng Wang
Cc: marcel, alex000young, johan.hedberg, luiz.dentz, davem, edumazet,
kuba, pabeni, linux-bluetooth, netdev, linux-kernel, pmenzel
friendly ping
Zheng Wang <zyytlz.wz@163.com> 于2023年2月17日周五 18:05写道：
>
> In hci_remove_adv_monitor, if it gets into HCI_ADV_MONITOR_EXT_MSFT case,
> the function will free the monitor and print its handle after that.
> Fix it by removing the logging into msft_le_cancel_monitor_advertisement_cb
> before calling hci_free_adv_monitor.
>
> Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
> ---
> v2:
> - move the logging inside msft_remove_monitor suggested by Luiz
> ---
> net/bluetooth/hci_core.c | 2 --
> net/bluetooth/msft.c | 2 ++
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index b65c3aabcd53..69b82c2907ff 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1981,8 +1981,6 @@ static int hci_remove_adv_monitor(struct hci_dev *hdev,
>
> case HCI_ADV_MONITOR_EXT_MSFT:
> status = msft_remove_monitor(hdev, monitor);
> - bt_dev_dbg(hdev, "%s remove monitor %d msft status %d",
> - hdev->name, monitor->handle, status);
> break;
> }
>
> diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
> index bee6a4c656be..4b35f0ed1360 100644
> --- a/net/bluetooth/msft.c
> +++ b/net/bluetooth/msft.c
> @@ -286,6 +286,8 @@ static int msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
> * suspend. It will be re-monitored on resume.
> */
> if (!msft->suspending) {
> + bt_dev_dbg(hdev, "%s remove monitor %d status %d", hdev->name,
> + monitor->handle, status);
> hci_free_adv_monitor(hdev, monitor);
>
> /* Clear any monitored devices by this Adv Monitor */
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Bluetooth: hci_core: Fix poential Use-after-Free bug in hci_remove_adv_monitor
2023-03-13 9:55 ` [PATCH v2] " Zheng Hacker
@ 2023-03-14 15:06 ` Simon Horman
2023-03-14 15:22 ` Simon Horman
0 siblings, 1 reply; 6+ messages in thread
From: Simon Horman @ 2023-03-14 15:06 UTC (permalink / raw)
To: Zheng Hacker
Cc: Zheng Wang, marcel, alex000young, johan.hedberg, luiz.dentz,
davem, edumazet, kuba, pabeni, linux-bluetooth, netdev,
linux-kernel, pmenzel
On Mon, Mar 13, 2023 at 05:55:35PM +0800, Zheng Hacker wrote:
> friendly ping
>
> Zheng Wang <zyytlz.wz@163.com> 于2023年2月17日周五 18:05写道：
> >
> > In hci_remove_adv_monitor, if it gets into HCI_ADV_MONITOR_EXT_MSFT case,
> > the function will free the monitor and print its handle after that.
> > Fix it by removing the logging into msft_le_cancel_monitor_advertisement_cb
> > before calling hci_free_adv_monitor.
> >
> > Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
> > ---
> > v2:
> > - move the logging inside msft_remove_monitor suggested by Luiz
> > ---
> > net/bluetooth/hci_core.c | 2 --
> > net/bluetooth/msft.c | 2 ++
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index b65c3aabcd53..69b82c2907ff 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -1981,8 +1981,6 @@ static int hci_remove_adv_monitor(struct hci_dev *hdev,
> >
> > case HCI_ADV_MONITOR_EXT_MSFT:
> > status = msft_remove_monitor(hdev, monitor);
> > - bt_dev_dbg(hdev, "%s remove monitor %d msft status %d",
> > - hdev->name, monitor->handle, status);
> > break;
I'm probably missing something obvious.
But from my perspective a simpler fix would be to
move the msft_remove_monitor() call to below the bt_dev_dbg() call.
> > }
> >
> > diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
> > index bee6a4c656be..4b35f0ed1360 100644
> > --- a/net/bluetooth/msft.c
> > +++ b/net/bluetooth/msft.c
> > @@ -286,6 +286,8 @@ static int msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
> > * suspend. It will be re-monitored on resume.
> > */
> > if (!msft->suspending) {
> > + bt_dev_dbg(hdev, "%s remove monitor %d status %d", hdev->name,
> > + monitor->handle, status);
> > hci_free_adv_monitor(hdev, monitor);
> >
> > /* Clear any monitored devices by this Adv Monitor */
> > --
> > 2.25.1
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Bluetooth: hci_core: Fix poential Use-after-Free bug in hci_remove_adv_monitor
2023-03-14 15:06 ` Simon Horman
@ 2023-03-14 15:22 ` Simon Horman
2023-03-15 9:01 ` Zheng Hacker
0 siblings, 1 reply; 6+ messages in thread
From: Simon Horman @ 2023-03-14 15:22 UTC (permalink / raw)
To: Zheng Hacker
Cc: Zheng Wang, marcel, alex000young, johan.hedberg, luiz.dentz,
davem, edumazet, kuba, pabeni, linux-bluetooth, netdev,
linux-kernel, pmenzel
On Tue, Mar 14, 2023 at 04:06:11PM +0100, Simon Horman wrote:
> On Mon, Mar 13, 2023 at 05:55:35PM +0800, Zheng Hacker wrote:
> > friendly ping
> >
> > Zheng Wang <zyytlz.wz@163.com> 于2023年2月17日周五 18:05写道：
> > >
> > > In hci_remove_adv_monitor, if it gets into HCI_ADV_MONITOR_EXT_MSFT case,
> > > the function will free the monitor and print its handle after that.
> > > Fix it by removing the logging into msft_le_cancel_monitor_advertisement_cb
> > > before calling hci_free_adv_monitor.
> > >
> > > Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
> > > ---
> > > v2:
> > > - move the logging inside msft_remove_monitor suggested by Luiz
> > > ---
> > > net/bluetooth/hci_core.c | 2 --
> > > net/bluetooth/msft.c | 2 ++
> > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > > index b65c3aabcd53..69b82c2907ff 100644
> > > --- a/net/bluetooth/hci_core.c
> > > +++ b/net/bluetooth/hci_core.c
> > > @@ -1981,8 +1981,6 @@ static int hci_remove_adv_monitor(struct hci_dev *hdev,
> > >
> > > case HCI_ADV_MONITOR_EXT_MSFT:
> > > status = msft_remove_monitor(hdev, monitor);
> > > - bt_dev_dbg(hdev, "%s remove monitor %d msft status %d",
> > > - hdev->name, monitor->handle, status);
> > > break;
>
> I'm probably missing something obvious.
> But from my perspective a simpler fix would be to
> move the msft_remove_monitor() call to below the bt_dev_dbg() call.
The obvious thing I was missing is that was what was done in v1
but Luiz suggested moving the logging to
msft_le_cancel_monitor_advertisement_cb().
Sorry for the noise.
Link: https://lore.kernel.org/all/CABBYNZL_gZ+kr_OEqjYgMmt+=91=jC88g310F-ScMC=kLh0xdw@mail.gmail.com/
>
> > > }
> > >
> > > diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
> > > index bee6a4c656be..4b35f0ed1360 100644
> > > --- a/net/bluetooth/msft.c
> > > +++ b/net/bluetooth/msft.c
> > > @@ -286,6 +286,8 @@ static int msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
> > > * suspend. It will be re-monitored on resume.
> > > */
> > > if (!msft->suspending) {
> > > + bt_dev_dbg(hdev, "%s remove monitor %d status %d", hdev->name,
> > > + monitor->handle, status);
> > > hci_free_adv_monitor(hdev, monitor);
> > >
> > > /* Clear any monitored devices by this Adv Monitor */
> > > --
> > > 2.25.1
> > >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Bluetooth: hci_core: Fix poential Use-after-Free bug in hci_remove_adv_monitor
2023-03-14 15:22 ` Simon Horman
@ 2023-03-15 9:01 ` Zheng Hacker
0 siblings, 0 replies; 6+ messages in thread
From: Zheng Hacker @ 2023-03-15 9:01 UTC (permalink / raw)
To: Simon Horman
Cc: Zheng Wang, marcel, alex000young, johan.hedberg, luiz.dentz,
davem, edumazet, kuba, pabeni, linux-bluetooth, netdev,
linux-kernel, pmenzel
Simon Horman <simon.horman@corigine.com> 于2023年3月14日周二 23:22写道：
>
> On Tue, Mar 14, 2023 at 04:06:11PM +0100, Simon Horman wrote:
> > On Mon, Mar 13, 2023 at 05:55:35PM +0800, Zheng Hacker wrote:
> > > friendly ping
> > >
> > > Zheng Wang <zyytlz.wz@163.com> 于2023年2月17日周五 18:05写道：
> > > >
> > > > In hci_remove_adv_monitor, if it gets into HCI_ADV_MONITOR_EXT_MSFT case,
> > > > the function will free the monitor and print its handle after that.
> > > > Fix it by removing the logging into msft_le_cancel_monitor_advertisement_cb
> > > > before calling hci_free_adv_monitor.
> > > >
> > > > Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
> > > > ---
> > > > v2:
> > > > - move the logging inside msft_remove_monitor suggested by Luiz
> > > > ---
> > > > net/bluetooth/hci_core.c | 2 --
> > > > net/bluetooth/msft.c | 2 ++
> > > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > > > index b65c3aabcd53..69b82c2907ff 100644
> > > > --- a/net/bluetooth/hci_core.c
> > > > +++ b/net/bluetooth/hci_core.c
> > > > @@ -1981,8 +1981,6 @@ static int hci_remove_adv_monitor(struct hci_dev *hdev,
> > > >
> > > > case HCI_ADV_MONITOR_EXT_MSFT:
> > > > status = msft_remove_monitor(hdev, monitor);
> > > > - bt_dev_dbg(hdev, "%s remove monitor %d msft status %d",
> > > > - hdev->name, monitor->handle, status);
> > > > break;
> >
> > I'm probably missing something obvious.
> > But from my perspective a simpler fix would be to
> > move the msft_remove_monitor() call to below the bt_dev_dbg() call.
>
> The obvious thing I was missing is that was what was done in v1
> but Luiz suggested moving the logging to
> msft_le_cancel_monitor_advertisement_cb().
> Sorry for the noise.
Hi Simon,
Thanks for your reply and detailed review :)
Best regards,
Zheng
>
> Link: https://lore.kernel.org/all/CABBYNZL_gZ+kr_OEqjYgMmt+=91=jC88g310F-ScMC=kLh0xdw@mail.gmail.com/
>
> >
> > > > }
> > > >
> > > > diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
> > > > index bee6a4c656be..4b35f0ed1360 100644
> > > > --- a/net/bluetooth/msft.c
> > > > +++ b/net/bluetooth/msft.c
> > > > @@ -286,6 +286,8 @@ static int msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
> > > > * suspend. It will be re-monitored on resume.
> > > > */
> > > > if (!msft->suspending) {
> > > > + bt_dev_dbg(hdev, "%s remove monitor %d status %d", hdev->name,
> > > > + monitor->handle, status);
> > > > hci_free_adv_monitor(hdev, monitor);
> > > >
> > > > /* Clear any monitored devices by this Adv Monitor */
> > > > --
> > > > 2.25.1
> > > >
> > >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-15 9:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-17 10:02 [PATCH v2] Bluetooth: hci_core: Fix poential Use-after-Free bug in hci_remove_adv_monitor Zheng Wang
2023-02-17 10:34 ` [v2] " bluez.test.bot
2023-03-13 9:55 ` [PATCH v2] " Zheng Hacker
2023-03-14 15:06 ` Simon Horman
2023-03-14 15:22 ` Simon Horman
2023-03-15 9:01 ` Zheng Hacker
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.