Re: [PATCH 1/1] Bluetooth: make the balance of judgement condition to fix a false report
Hi Zumeng, > This patch is to balance the condition scope between hci_get_cmd_complete and > hci_event_packet about orig_skb as follows: > >if (req_complete_skb || event == HCI_EV_CMD_STATUS || >event == HCI_EV_CMD_COMPLETE) >orig_skb = skb_clone(skb, GFP_KERNEL); > > And hci_get_cmd_complete will bt_dev_err out when HCI_EV_CMD_STATUS, so a lot > of asymmetric conditions are triggered. Since both of them are the entry into > hci_get_cmd_complete, we'd better get STATUS into judge the false out there. > > Signed-off-by: Zumeng Chen > --- > > Hi expert, > > This issue existed whether or not T_DBG had been changed into bt_dev_err, > which > just shows the issue explicitly. I noticed actually that opcode doesn't match > ev->opcode either at the same time. And there might be some logic issue about > HCI_EV_CMD_COMPLETE between protocol and drivers. I'm not familar with the > whole > bluetooth protocol, and not gonna to dig more, so all yours guys~ > > Cheers, > Zumeng > > net/bluetooth/hci_event.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > index 235b5aa..d848663 100644 > --- a/net/bluetooth/hci_event.c > +++ b/net/bluetooth/hci_event.c > @@ -5217,7 +5217,8 @@ static bool hci_get_cmd_complete(struct hci_dev *hdev, > u16 opcode, > return true; > } > > - if (hdr->evt != HCI_EV_CMD_COMPLETE) { > + if (!((hdr->evt == HCI_EV_CMD_COMPLETE) || > + (hdr->evt == HCI_EV_CMD_STATUS))) { this indentation is messed up. Also some braces are not needed. if (!(hdr->evt == HCI_EV_CMD_COMPLETE || hdr->evt == HCI_EV_CMD_STATUS)) { > bt_dev_err(hdev, "last event is not cmd complete (0x%2.2x)", > hdr->evt); > return false; Regards Marcel
Re: [PATCH 1/1] Bluetooth: make the balance of judgement condition to fix a false report
Hi Zumeng, > This patch is to balance the condition scope between hci_get_cmd_complete and > hci_event_packet about orig_skb as follows: > >if (req_complete_skb || event == HCI_EV_CMD_STATUS || >event == HCI_EV_CMD_COMPLETE) >orig_skb = skb_clone(skb, GFP_KERNEL); > > And hci_get_cmd_complete will bt_dev_err out when HCI_EV_CMD_STATUS, so a lot > of asymmetric conditions are triggered. Since both of them are the entry into > hci_get_cmd_complete, we'd better get STATUS into judge the false out there. > > Signed-off-by: Zumeng Chen > --- > > Hi expert, > > This issue existed whether or not T_DBG had been changed into bt_dev_err, > which > just shows the issue explicitly. I noticed actually that opcode doesn't match > ev->opcode either at the same time. And there might be some logic issue about > HCI_EV_CMD_COMPLETE between protocol and drivers. I'm not familar with the > whole > bluetooth protocol, and not gonna to dig more, so all yours guys~ > > Cheers, > Zumeng > > net/bluetooth/hci_event.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > index 235b5aa..d848663 100644 > --- a/net/bluetooth/hci_event.c > +++ b/net/bluetooth/hci_event.c > @@ -5217,7 +5217,8 @@ static bool hci_get_cmd_complete(struct hci_dev *hdev, > u16 opcode, > return true; > } > > - if (hdr->evt != HCI_EV_CMD_COMPLETE) { > + if (!((hdr->evt == HCI_EV_CMD_COMPLETE) || > + (hdr->evt == HCI_EV_CMD_STATUS))) { this indentation is messed up. Also some braces are not needed. if (!(hdr->evt == HCI_EV_CMD_COMPLETE || hdr->evt == HCI_EV_CMD_STATUS)) { > bt_dev_err(hdev, "last event is not cmd complete (0x%2.2x)", > hdr->evt); > return false; Regards Marcel
[PATCH 1/1] Bluetooth: make the balance of judgement condition to fix a false report
This patch is to balance the condition scope between hci_get_cmd_complete and hci_event_packet about orig_skb as follows: if (req_complete_skb || event == HCI_EV_CMD_STATUS || event == HCI_EV_CMD_COMPLETE) orig_skb = skb_clone(skb, GFP_KERNEL); And hci_get_cmd_complete will bt_dev_err out when HCI_EV_CMD_STATUS, so a lot of asymmetric conditions are triggered. Since both of them are the entry into hci_get_cmd_complete, we'd better get STATUS into judge the false out there. Signed-off-by: Zumeng Chen --- Hi expert, This issue existed whether or not T_DBG had been changed into bt_dev_err, which just shows the issue explicitly. I noticed actually that opcode doesn't match ev->opcode either at the same time. And there might be some logic issue about HCI_EV_CMD_COMPLETE between protocol and drivers. I'm not familar with the whole bluetooth protocol, and not gonna to dig more, so all yours guys~ Cheers, Zumeng net/bluetooth/hci_event.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 235b5aa..d848663 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -5217,7 +5217,8 @@ static bool hci_get_cmd_complete(struct hci_dev *hdev, u16 opcode, return true; } - if (hdr->evt != HCI_EV_CMD_COMPLETE) { + if (!((hdr->evt == HCI_EV_CMD_COMPLETE) || + (hdr->evt == HCI_EV_CMD_STATUS))) { bt_dev_err(hdev, "last event is not cmd complete (0x%2.2x)", hdr->evt); return false; -- 2.7.4
[PATCH 1/1] Bluetooth: make the balance of judgement condition to fix a false report
This patch is to balance the condition scope between hci_get_cmd_complete and hci_event_packet about orig_skb as follows: if (req_complete_skb || event == HCI_EV_CMD_STATUS || event == HCI_EV_CMD_COMPLETE) orig_skb = skb_clone(skb, GFP_KERNEL); And hci_get_cmd_complete will bt_dev_err out when HCI_EV_CMD_STATUS, so a lot of asymmetric conditions are triggered. Since both of them are the entry into hci_get_cmd_complete, we'd better get STATUS into judge the false out there. Signed-off-by: Zumeng Chen --- Hi expert, This issue existed whether or not T_DBG had been changed into bt_dev_err, which just shows the issue explicitly. I noticed actually that opcode doesn't match ev->opcode either at the same time. And there might be some logic issue about HCI_EV_CMD_COMPLETE between protocol and drivers. I'm not familar with the whole bluetooth protocol, and not gonna to dig more, so all yours guys~ Cheers, Zumeng net/bluetooth/hci_event.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 235b5aa..d848663 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -5217,7 +5217,8 @@ static bool hci_get_cmd_complete(struct hci_dev *hdev, u16 opcode, return true; } - if (hdr->evt != HCI_EV_CMD_COMPLETE) { + if (!((hdr->evt == HCI_EV_CMD_COMPLETE) || + (hdr->evt == HCI_EV_CMD_STATUS))) { bt_dev_err(hdev, "last event is not cmd complete (0x%2.2x)", hdr->evt); return false; -- 2.7.4