| Message ID | 20230217100223.702330-1-zyytlz.wz@163.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] Bluetooth: hci_core: Fix poential Use-after-Free bug in hci_remove_adv_monitor | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | fail | 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" |
| tedd_an/SubjectPrefix | success | Gitlint PASS |
| tedd_an/BuildKernel | success | BuildKernel PASS |
| tedd_an/CheckAllWarning | success | CheckAllWarning PASS |
| tedd_an/CheckSparse | success | CheckSparse PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/BuildKernel32 | success | BuildKernel32 PASS |
| tedd_an/TestRunnerSetup | success | TestRunnerSetup PASS |
| tedd_an/TestRunner_l2cap-tester | success | TestRunner PASS |
| tedd_an/TestRunner_iso-tester | success | TestRunner PASS |
| tedd_an/TestRunner_bnep-tester | success | TestRunner PASS |
| tedd_an/TestRunner_mgmt-tester | success | TestRunner PASS |
| tedd_an/TestRunner_rfcomm-tester | success | TestRunner PASS |
| tedd_an/TestRunner_sco-tester | success | TestRunner PASS |
| tedd_an/TestRunner_ioctl-tester | success | TestRunner PASS |
| tedd_an/TestRunner_mesh-tester | success | TestRunner PASS |
| tedd_an/TestRunner_smp-tester | success | TestRunner PASS |
| tedd_an/TestRunner_userchan-tester | success | TestRunner PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
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
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 >
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 > > >
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 > > > > >
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 > > > > > > >
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 */
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(-)