* [Linux-kernel-mentees] [PATCH] net: bluetooth: Fix null pointer deref in hci_phy_link_complete_evt @ 2020-08-29 12:41 Anmol Karn 2020-08-29 16:57 ` Anmol Karn 0 siblings, 1 reply; 7+ messages in thread From: Anmol Karn @ 2020-08-29 12:41 UTC (permalink / raw) To: marcel, johan.hedberg Cc: netdev, syzbot+0bef568258653cff272f, syzkaller-bugs, linux-kernel, linux-bluetooth, kuba, linux-kernel-mentees, davem Fix null pointer deref in hci_phy_link_complete_evt, there was no checking there for the hcon->amp_mgr->l2cap_conn->hconn, and also in hci_cmd_work, for hdev->sent_cmd. To fix this issue Add pointer checking in hci_cmd_work and hci_phy_link_complete_evt. [Linux-next-20200827] Reported-by: syzbot+0bef568258653cff272f@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?id=0d93140da5a82305a66a136af99b088b75177b99 Signed-off-by: Anmol Karn <anmol.karan123@gmail.com> --- net/bluetooth/hci_core.c | 4 ++++ net/bluetooth/hci_event.c | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 68bfe57b6625..533048d2a624 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -4922,6 +4922,10 @@ static void hci_cmd_work(struct work_struct *work) kfree_skb(hdev->sent_cmd); + if(hdev->sent_cmd) { + hdev->sent_cmd = skb_clone(skb, GFP_KERNEL); + } + hdev->sent_cmd = skb_clone(skb, GFP_KERNEL); if (hdev->sent_cmd) { if (hci_req_status_pend(hdev)) diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 4b7fc430793c..c621c8a20ea4 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -4941,6 +4941,10 @@ static void hci_phy_link_complete_evt(struct hci_dev *hdev, hci_dev_unlock(hdev); return; } + if(!(hcon->amp_mgr->l2cap_conn->hcon)) { + hci_dev_unlock(hdev); + return; + } bredr_hcon = hcon->amp_mgr->l2cap_conn->hcon; -- 2.28.0 _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Linux-kernel-mentees] [PATCH] net: bluetooth: Fix null pointer deref in hci_phy_link_complete_evt 2020-08-29 12:41 [Linux-kernel-mentees] [PATCH] net: bluetooth: Fix null pointer deref in hci_phy_link_complete_evt Anmol Karn @ 2020-08-29 16:57 ` Anmol Karn 2020-08-30 9:18 ` Greg KH 2020-08-30 9:19 ` Greg KH 0 siblings, 2 replies; 7+ messages in thread From: Anmol Karn @ 2020-08-29 16:57 UTC (permalink / raw) To: marcel, johan.hedberg Cc: netdev, syzbot+0bef568258653cff272f, syzkaller-bugs, linux-kernel, linux-bluetooth, kuba, linux-kernel-mentees, davem Fix null pointer deref in hci_phy_link_complete_evt, there was no checking there for the hcon->amp_mgr->l2cap_conn->hconn, and also in hci_cmd_work, for hdev->sent_cmd. To fix this issue Add pointer checking in hci_cmd_work and hci_phy_link_complete_evt. [Linux-next-20200827] This patch corrected some mistakes from previous patch. Reported-by: syzbot+0bef568258653cff272f@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?id=0d93140da5a82305a66a136af99b088b75177b99 Signed-off-by: Anmol Karn <anmol.karan123@gmail.com> --- net/bluetooth/hci_core.c | 5 ++++- net/bluetooth/hci_event.c | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 68bfe57b6625..996efd654e7a 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -4922,7 +4922,10 @@ static void hci_cmd_work(struct work_struct *work) kfree_skb(hdev->sent_cmd); - hdev->sent_cmd = skb_clone(skb, GFP_KERNEL); + if (hdev->sent_cmd) { + hdev->sent_cmd = skb_clone(skb, GFP_KERNEL); + } + if (hdev->sent_cmd) { if (hci_req_status_pend(hdev)) hci_dev_set_flag(hdev, HCI_CMD_PENDING); diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 4b7fc430793c..1e7d9bee9111 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -4941,6 +4941,10 @@ static void hci_phy_link_complete_evt(struct hci_dev *hdev, hci_dev_unlock(hdev); return; } + if (!(hcon->amp_mgr->l2cap_conn->hcon)) { + hci_dev_unlock(hdev); + return; + } bredr_hcon = hcon->amp_mgr->l2cap_conn->hcon; -- 2.28.0 _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] net: bluetooth: Fix null pointer deref in hci_phy_link_complete_evt 2020-08-29 16:57 ` Anmol Karn @ 2020-08-30 9:18 ` Greg KH 2020-08-30 9:19 ` Greg KH 1 sibling, 0 replies; 7+ messages in thread From: Greg KH @ 2020-08-30 9:18 UTC (permalink / raw) To: Anmol Karn Cc: johan.hedberg, netdev, marcel, syzkaller-bugs, linux-kernel, davem, linux-bluetooth, kuba, linux-kernel-mentees, syzbot+0bef568258653cff272f On Sat, Aug 29, 2020 at 10:27:12PM +0530, Anmol Karn wrote: > Fix null pointer deref in hci_phy_link_complete_evt, there was no > checking there for the hcon->amp_mgr->l2cap_conn->hconn, and also > in hci_cmd_work, for hdev->sent_cmd. > > To fix this issue Add pointer checking in hci_cmd_work and > hci_phy_link_complete_evt. > [Linux-next-20200827] > > This patch corrected some mistakes from previous patch. So this is a v2? That should go below the --- line, right? And you should have a v2 in the subject line like the documentation asks? Can you redo this please? thanks, greg k-h _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] net: bluetooth: Fix null pointer deref in hci_phy_link_complete_evt 2020-08-29 16:57 ` Anmol Karn 2020-08-30 9:18 ` Greg KH @ 2020-08-30 9:19 ` Greg KH 2020-08-30 12:26 ` Anmol Karn 1 sibling, 1 reply; 7+ messages in thread From: Greg KH @ 2020-08-30 9:19 UTC (permalink / raw) To: Anmol Karn Cc: johan.hedberg, netdev, marcel, syzkaller-bugs, linux-kernel, davem, linux-bluetooth, kuba, linux-kernel-mentees, syzbot+0bef568258653cff272f On Sat, Aug 29, 2020 at 10:27:12PM +0530, Anmol Karn wrote: > Fix null pointer deref in hci_phy_link_complete_evt, there was no > checking there for the hcon->amp_mgr->l2cap_conn->hconn, and also > in hci_cmd_work, for hdev->sent_cmd. > > To fix this issue Add pointer checking in hci_cmd_work and > hci_phy_link_complete_evt. > [Linux-next-20200827] > > This patch corrected some mistakes from previous patch. > > Reported-by: syzbot+0bef568258653cff272f@syzkaller.appspotmail.com > Link: https://syzkaller.appspot.com/bug?id=0d93140da5a82305a66a136af99b088b75177b99 > Signed-off-by: Anmol Karn <anmol.karan123@gmail.com> > --- > net/bluetooth/hci_core.c | 5 ++++- > net/bluetooth/hci_event.c | 4 ++++ > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > index 68bfe57b6625..996efd654e7a 100644 > --- a/net/bluetooth/hci_core.c > +++ b/net/bluetooth/hci_core.c > @@ -4922,7 +4922,10 @@ static void hci_cmd_work(struct work_struct *work) > > kfree_skb(hdev->sent_cmd); > > - hdev->sent_cmd = skb_clone(skb, GFP_KERNEL); > + if (hdev->sent_cmd) { > + hdev->sent_cmd = skb_clone(skb, GFP_KERNEL); > + } How can sent_cmd be NULL here? Are you sure something previous to this shouldn't be fixed instead? > + > if (hdev->sent_cmd) { > if (hci_req_status_pend(hdev)) > hci_dev_set_flag(hdev, HCI_CMD_PENDING); > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > index 4b7fc430793c..1e7d9bee9111 100644 > --- a/net/bluetooth/hci_event.c > +++ b/net/bluetooth/hci_event.c > @@ -4941,6 +4941,10 @@ static void hci_phy_link_complete_evt(struct hci_dev *hdev, > hci_dev_unlock(hdev); > return; > } > + if (!(hcon->amp_mgr->l2cap_conn->hcon)) { > + hci_dev_unlock(hdev); > + return; > + } How can this be triggered? thanks, greg k-h _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] net: bluetooth: Fix null pointer deref in hci_phy_link_complete_evt 2020-08-30 9:19 ` Greg KH @ 2020-08-30 12:26 ` Anmol Karn 2020-08-30 17:30 ` Greg KH 0 siblings, 1 reply; 7+ messages in thread From: Anmol Karn @ 2020-08-30 12:26 UTC (permalink / raw) To: Greg KH Cc: netdev, syzbot+0bef568258653cff272f, syzkaller-bugs, linux-kernel, linux-bluetooth, kuba, linux-kernel-mentees, davem On Sun, Aug 30, 2020 at 11:19:17AM +0200, Greg KH wrote: > On Sat, Aug 29, 2020 at 10:27:12PM +0530, Anmol Karn wrote: > > Fix null pointer deref in hci_phy_link_complete_evt, there was no > > checking there for the hcon->amp_mgr->l2cap_conn->hconn, and also > > in hci_cmd_work, for hdev->sent_cmd. > > > > To fix this issue Add pointer checking in hci_cmd_work and > > hci_phy_link_complete_evt. > > [Linux-next-20200827] > > > > This patch corrected some mistakes from previous patch. > > > > Reported-by: syzbot+0bef568258653cff272f@syzkaller.appspotmail.com > > Link: https://syzkaller.appspot.com/bug?id=0d93140da5a82305a66a136af99b088b75177b99 > > Signed-off-by: Anmol Karn <anmol.karan123@gmail.com> > > --- > > net/bluetooth/hci_core.c | 5 ++++- > > net/bluetooth/hci_event.c | 4 ++++ > > 2 files changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > > index 68bfe57b6625..996efd654e7a 100644 > > --- a/net/bluetooth/hci_core.c > > +++ b/net/bluetooth/hci_core.c > > @@ -4922,7 +4922,10 @@ static void hci_cmd_work(struct work_struct *work) > > > > kfree_skb(hdev->sent_cmd); > > > > - hdev->sent_cmd = skb_clone(skb, GFP_KERNEL); > > + if (hdev->sent_cmd) { > > + hdev->sent_cmd = skb_clone(skb, GFP_KERNEL); > > + } > > How can sent_cmd be NULL here? Are you sure something previous to this > shouldn't be fixed instead? Sir, sent_cmd was freed before this condition check, thats why i checked it, i think i should check it before the free of hdev->sent_cmd like, if (hdev->sent_cmd) kfree_skb(hdev->sent_cmd); what's your opininon on this. > > > > + > > if (hdev->sent_cmd) { > > if (hci_req_status_pend(hdev)) > > hci_dev_set_flag(hdev, HCI_CMD_PENDING); > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > > index 4b7fc430793c..1e7d9bee9111 100644 > > --- a/net/bluetooth/hci_event.c > > +++ b/net/bluetooth/hci_event.c > > @@ -4941,6 +4941,10 @@ static void hci_phy_link_complete_evt(struct hci_dev *hdev, > > hci_dev_unlock(hdev); > > return; > > } > > + if (!(hcon->amp_mgr->l2cap_conn->hcon)) { > > + hci_dev_unlock(hdev); > > + return; > > + } > > How can this be triggered? syzbot showed that this line is accessed irrespective of the null value it contains, so added a pointer check for that. please give some opinions on this, if (!bredr_hcon) { hci_dev_unlock(hdev); return; } Thanks, Anmol Karn _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] net: bluetooth: Fix null pointer deref in hci_phy_link_complete_evt 2020-08-30 12:26 ` Anmol Karn @ 2020-08-30 17:30 ` Greg KH 2020-08-30 20:42 ` Anmol Karn 0 siblings, 1 reply; 7+ messages in thread From: Greg KH @ 2020-08-30 17:30 UTC (permalink / raw) To: Anmol Karn Cc: netdev, syzbot+0bef568258653cff272f, syzkaller-bugs, linux-kernel, linux-bluetooth, kuba, linux-kernel-mentees, davem On Sun, Aug 30, 2020 at 05:56:23PM +0530, Anmol Karn wrote: > On Sun, Aug 30, 2020 at 11:19:17AM +0200, Greg KH wrote: > > On Sat, Aug 29, 2020 at 10:27:12PM +0530, Anmol Karn wrote: > > > Fix null pointer deref in hci_phy_link_complete_evt, there was no > > > checking there for the hcon->amp_mgr->l2cap_conn->hconn, and also > > > in hci_cmd_work, for hdev->sent_cmd. > > > > > > To fix this issue Add pointer checking in hci_cmd_work and > > > hci_phy_link_complete_evt. > > > [Linux-next-20200827] > > > > > > This patch corrected some mistakes from previous patch. > > > > > > Reported-by: syzbot+0bef568258653cff272f@syzkaller.appspotmail.com > > > Link: https://syzkaller.appspot.com/bug?id=0d93140da5a82305a66a136af99b088b75177b99 > > > Signed-off-by: Anmol Karn <anmol.karan123@gmail.com> > > > --- > > > net/bluetooth/hci_core.c | 5 ++++- > > > net/bluetooth/hci_event.c | 4 ++++ > > > 2 files changed, 8 insertions(+), 1 deletion(-) > > > > > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > > > index 68bfe57b6625..996efd654e7a 100644 > > > --- a/net/bluetooth/hci_core.c > > > +++ b/net/bluetooth/hci_core.c > > > @@ -4922,7 +4922,10 @@ static void hci_cmd_work(struct work_struct *work) > > > > > > kfree_skb(hdev->sent_cmd); > > > > > > - hdev->sent_cmd = skb_clone(skb, GFP_KERNEL); > > > + if (hdev->sent_cmd) { > > > + hdev->sent_cmd = skb_clone(skb, GFP_KERNEL); > > > + } > > > > How can sent_cmd be NULL here? Are you sure something previous to this > > shouldn't be fixed instead? > > Sir, sent_cmd was freed before this condition check, thats why i checked it, But it can not be NULL at that point in time, as nothing set it to NULL, correct? > i think i should check it before the free of hdev->sent_cmd like, > > if (hdev->sent_cmd) > kfree_skb(hdev->sent_cmd); No, that's not needed. What is the problem with these lines that you are trying to solve? > > > + > > > if (hdev->sent_cmd) { > > > if (hci_req_status_pend(hdev)) > > > hci_dev_set_flag(hdev, HCI_CMD_PENDING); > > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > > > index 4b7fc430793c..1e7d9bee9111 100644 > > > --- a/net/bluetooth/hci_event.c > > > +++ b/net/bluetooth/hci_event.c > > > @@ -4941,6 +4941,10 @@ static void hci_phy_link_complete_evt(struct hci_dev *hdev, > > > hci_dev_unlock(hdev); > > > return; > > > } > > > + if (!(hcon->amp_mgr->l2cap_conn->hcon)) { > > > + hci_dev_unlock(hdev); > > > + return; > > > + } > > > > How can this be triggered? > > syzbot showed that this line is accessed irrespective of the null value it contains, so added a > pointer check for that. But does hcon->amp_mgr->l2cap_conn->hcon become NULL here? thanks, greg k-h _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] net: bluetooth: Fix null pointer deref in hci_phy_link_complete_evt 2020-08-30 17:30 ` Greg KH @ 2020-08-30 20:42 ` Anmol Karn 0 siblings, 0 replies; 7+ messages in thread From: Anmol Karn @ 2020-08-30 20:42 UTC (permalink / raw) To: Greg KH Cc: netdev, syzbot+0bef568258653cff272f, syzkaller-bugs, linux-kernel, linux-bluetooth, kuba, linux-kernel-mentees, davem On Sun, Aug 30, 2020 at 07:30:10PM +0200, Greg KH wrote: > On Sun, Aug 30, 2020 at 05:56:23PM +0530, Anmol Karn wrote: > > On Sun, Aug 30, 2020 at 11:19:17AM +0200, Greg KH wrote: > > > On Sat, Aug 29, 2020 at 10:27:12PM +0530, Anmol Karn wrote: > > > > Fix null pointer deref in hci_phy_link_complete_evt, there was no > > > > checking there for the hcon->amp_mgr->l2cap_conn->hconn, and also > > > > in hci_cmd_work, for hdev->sent_cmd. > > > > > > > > To fix this issue Add pointer checking in hci_cmd_work and > > > > hci_phy_link_complete_evt. > > > > [Linux-next-20200827] > > > > > > > > This patch corrected some mistakes from previous patch. > > > > > > > > Reported-by: syzbot+0bef568258653cff272f@syzkaller.appspotmail.com > > > > Link: https://syzkaller.appspot.com/bug?id=0d93140da5a82305a66a136af99b088b75177b99 > > > > Signed-off-by: Anmol Karn <anmol.karan123@gmail.com> > > > > --- > > > > net/bluetooth/hci_core.c | 5 ++++- > > > > net/bluetooth/hci_event.c | 4 ++++ > > > > 2 files changed, 8 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > > > > index 68bfe57b6625..996efd654e7a 100644 > > > > --- a/net/bluetooth/hci_core.c > > > > +++ b/net/bluetooth/hci_core.c > > > > @@ -4922,7 +4922,10 @@ static void hci_cmd_work(struct work_struct *work) > > > > > > > > kfree_skb(hdev->sent_cmd); > > > > > > > > - hdev->sent_cmd = skb_clone(skb, GFP_KERNEL); > > > > + if (hdev->sent_cmd) { > > > > + hdev->sent_cmd = skb_clone(skb, GFP_KERNEL); > > > > + } > > > > > > How can sent_cmd be NULL here? Are you sure something previous to this > > > shouldn't be fixed instead? > > > > Sir, sent_cmd was freed before this condition check, thats why i checked it, > > But it can not be NULL at that point in time, as nothing set it to NULL, > correct? > > > i think i should check it before the free of hdev->sent_cmd like, > > > > if (hdev->sent_cmd) > > kfree_skb(hdev->sent_cmd); > > No, that's not needed. > > What is the problem with these lines that you are trying to solve? > > > > > + > > > > if (hdev->sent_cmd) { > > > > if (hci_req_status_pend(hdev)) > > > > hci_dev_set_flag(hdev, HCI_CMD_PENDING); > > > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > > > > index 4b7fc430793c..1e7d9bee9111 100644 > > > > --- a/net/bluetooth/hci_event.c > > > > +++ b/net/bluetooth/hci_event.c > > > > @@ -4941,6 +4941,10 @@ static void hci_phy_link_complete_evt(struct hci_dev *hdev, > > > > hci_dev_unlock(hdev); > > > > return; > > > > } > > > > + if (!(hcon->amp_mgr->l2cap_conn->hcon)) { > > > > + hci_dev_unlock(hdev); > > > > + return; > > > > + } > > > > > > How can this be triggered? > > > > syzbot showed that this line is accessed irrespective of the null value it contains, so added a > > pointer check for that. > > But does hcon->amp_mgr->l2cap_conn->hcon become NULL here? Sir, according to the report obtained by running decode_stacktrace on logs there is something getting null at this line, after verifying the buggy address i thought it would be better to check this whole line. will dig more deeper into this and will make appropriate changes in the next version, thanks for review. Anmol Karn _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-08-30 20:42 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-08-29 12:41 [Linux-kernel-mentees] [PATCH] net: bluetooth: Fix null pointer deref in hci_phy_link_complete_evt Anmol Karn 2020-08-29 16:57 ` Anmol Karn 2020-08-30 9:18 ` Greg KH 2020-08-30 9:19 ` Greg KH 2020-08-30 12:26 ` Anmol Karn 2020-08-30 17:30 ` Greg KH 2020-08-30 20:42 ` Anmol Karn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).