2021-04-21 00:09:05

by Tedd Ho-Jeong An

[permalink] [raw]
Subject: [BlueZ v3] monitor: Fix the incorrect vendor name

From: Tedd Ho-Jeong An <[email protected]>

This patch fixes the vendor name is alwasy shown as "Microsoft" for the
commands and Unknown name for vendor events.

< HCI Command: Microsoft Secure Send (0x3f|0x0009) plen 249
Type: Data fragment (0x01)
> HCI Event: Command Complete (0x0e) plen 4
Microsoft Secure Send (0x3f|0x0009) ncmd 31
Status: Success (0x00)
---
monitor/msft.h | 6 +++
monitor/packet.c | 132 ++++++++++++++++++++++++++++++++++++++---------
2 files changed, 114 insertions(+), 24 deletions(-)

diff --git a/monitor/msft.h b/monitor/msft.h
index a268f4bc7..9072c6ef8 100644
--- a/monitor/msft.h
+++ b/monitor/msft.h
@@ -24,6 +24,12 @@

#include <stdint.h>

+struct msft_ext {
+ uint16_t opcode;
+ uint8_t evt_prefix_len;
+ uint8_t evt_prefix[32];
+} __attribute__ ((packed));
+
struct vendor_ocf;
struct vendor_evt;

diff --git a/monitor/packet.c b/monitor/packet.c
index d729a01cc..e43470e81 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -265,7 +265,7 @@ struct index_data {
uint8_t type;
uint8_t bdaddr[6];
uint16_t manufacturer;
- uint16_t msft_opcode;
+ struct msft_ext msft_ext;
size_t frame;
};

@@ -3939,7 +3939,9 @@ void packet_monitor(struct timeval *tv, struct ucred *cred,
index_list[index].type = ni->type;
memcpy(index_list[index].bdaddr, ni->bdaddr, 6);
index_list[index].manufacturer = fallback_manufacturer;
- index_list[index].msft_opcode = BT_HCI_CMD_NOP;
+ index_list[index].msft_ext.opcode = BT_HCI_CMD_NOP;
+ index_list[index].msft_ext.evt_prefix_len = 0;
+ memset(index_list[index].msft_ext.evt_prefix, 0, 32);
}

addr2str(ni->bdaddr, str);
@@ -4006,9 +4008,12 @@ void packet_monitor(struct timeval *tv, struct ucred *cred,
/*
* Intel controllers that support the
* Microsoft vendor extension are using
- * 0xFC1E for VsMsftOpCode.
+ * 0xFC1E for VsMsftOpCode and 0x50 for event
+ * code.
*/
- index_list[index].msft_opcode = 0xFC1E;
+ index_list[index].msft_ext.opcode = 0xFC1E;
+ index_list[index].msft_ext.evt_prefix_len = 1;
+ index_list[index].msft_ext.evt_prefix[0] = 0x50;
break;
case 93:
/*
@@ -4016,7 +4021,16 @@ void packet_monitor(struct timeval *tv, struct ucred *cred,
* Microsoft vendor extenions are using
* 0xFCF0 for VsMsftOpCode.
*/
- index_list[index].msft_opcode = 0xFCF0;
+ index_list[index].msft_ext.opcode = 0xFCF0;
+ index_list[index].msft_ext.evt_prefix_len = 8;
+ index_list[index].msft_ext.evt_prefix[0] = 0x23;
+ index_list[index].msft_ext.evt_prefix[1] = 0x79;
+ index_list[index].msft_ext.evt_prefix[2] = 0x54;
+ index_list[index].msft_ext.evt_prefix[3] = 0x33;
+ index_list[index].msft_ext.evt_prefix[4] = 0x77;
+ index_list[index].msft_ext.evt_prefix[5] = 0x88;
+ index_list[index].msft_ext.evt_prefix[6] = 0x97;
+ index_list[index].msft_ext.evt_prefix[7] = 0x68;
break;
}
}
@@ -9323,19 +9337,20 @@ static const char *get_supported_command(int bit)
return NULL;
}

-static const char *current_vendor_str(void)
+static const char *current_vendor_str(uint16_t ocf)
{
uint16_t manufacturer, msft_opcode;

if (index_current < MAX_INDEX) {
manufacturer = index_list[index_current].manufacturer;
- msft_opcode = index_list[index_current].msft_opcode;
+ msft_opcode = index_list[index_current].msft_ext.opcode;
} else {
manufacturer = fallback_manufacturer;
msft_opcode = BT_HCI_CMD_NOP;
}

- if (msft_opcode != BT_HCI_CMD_NOP)
+ if (msft_opcode != BT_HCI_CMD_NOP &&
+ cmd_opcode_ocf(msft_opcode) == ocf)
return "Microsoft";

switch (manufacturer) {
@@ -9350,13 +9365,34 @@ static const char *current_vendor_str(void)
return NULL;
}

+static const char *current_vendor_evt_str(uint8_t evt)
+{
+ uint16_t manufacturer;
+
+ if (index_current < MAX_INDEX)
+ manufacturer = index_list[index_current].manufacturer;
+ else
+ manufacturer = fallback_manufacturer;
+
+ switch (manufacturer) {
+ case 2:
+ return "Intel";
+ case 15:
+ return "Broadcom";
+ case 93:
+ return "Realtek";
+ }
+
+ return NULL;
+}
+
static const struct vendor_ocf *current_vendor_ocf(uint16_t ocf)
{
uint16_t manufacturer, msft_opcode;

if (index_current < MAX_INDEX) {
manufacturer = index_list[index_current].manufacturer;
- msft_opcode = index_list[index_current].msft_opcode;
+ msft_opcode = index_list[index_current].msft_ext.opcode;
} else {
manufacturer = fallback_manufacturer;
msft_opcode = BT_HCI_CMD_NOP;
@@ -9378,18 +9414,12 @@ static const struct vendor_ocf *current_vendor_ocf(uint16_t ocf)

static const struct vendor_evt *current_vendor_evt(uint8_t evt)
{
- uint16_t manufacturer, msft_opcode;
+ uint16_t manufacturer;

- if (index_current < MAX_INDEX) {
+ if (index_current < MAX_INDEX)
manufacturer = index_list[index_current].manufacturer;
- msft_opcode = index_list[index_current].msft_opcode;
- } else {
+ else
manufacturer = fallback_manufacturer;
- msft_opcode = BT_HCI_CMD_NOP;
- }
-
- if (msft_opcode != BT_HCI_CMD_NOP)
- return NULL;

switch (manufacturer) {
case 2:
@@ -9401,6 +9431,27 @@ static const struct vendor_evt *current_vendor_evt(uint8_t evt)
return NULL;
}

+static const struct vendor_evt *current_vendor_msft_evt(const void *data,
+ uint8_t size)
+{
+ uint8_t *prefix, prefix_len;
+
+ if (index_current < MAX_INDEX) {
+ prefix_len = index_list[index_current].msft_ext.evt_prefix_len;
+ prefix = index_list[index_current].msft_ext.evt_prefix;
+ } else
+ return NULL;
+
+ /* MSFT extension events start with the MSFT event prefix which is
+ * defined by the vendor and followed by the MSFT event code.
+ */
+ if (size > prefix_len && !memcmp(data, prefix, prefix_len)) {
+ return msft_vendor_evt();
+ }
+
+ return NULL;
+}
+
static void inquiry_complete_evt(const void *data, uint8_t size)
{
const struct bt_hci_evt_inquiry_complete *evt = data;
@@ -9573,7 +9624,7 @@ static void cmd_complete_evt(const void *data, uint8_t size)
const struct vendor_ocf *vnd = current_vendor_ocf(ocf);

if (vnd) {
- const char *str = current_vendor_str();
+ const char *str = current_vendor_str(ocf);

if (str) {
snprintf(vendor_str, sizeof(vendor_str),
@@ -9665,7 +9716,7 @@ static void cmd_status_evt(const void *data, uint8_t size)
const struct vendor_ocf *vnd = current_vendor_ocf(ocf);

if (vnd) {
- const char *str = current_vendor_str();
+ const char *str = current_vendor_str(ocf);

if (str) {
snprintf(vendor_str, sizeof(vendor_str),
@@ -11012,13 +11063,46 @@ static void le_meta_event_evt(const void *data, uint8_t size)

static void vendor_evt(const void *data, uint8_t size)
{
- uint8_t subevent = *((const uint8_t *) data);
+ uint8_t subevent;
struct subevent_data vendor_data;
char vendor_str[150];
- const struct vendor_evt *vnd = current_vendor_evt(subevent);
+ const struct vendor_evt *vnd;
+
+ /* For MSFT Extension event only */
+ vnd = current_vendor_msft_evt(data, size);
+ if (vnd) {
+ snprintf(vendor_str, sizeof(vendor_str),
+ "Microsoft %s", vnd->str);
+ print_indent(6, COLOR_HCI_EVENT, "", vendor_str, COLOR_OFF,
+ " length: %u", size);
+
+ if (!vnd->evt_func) {
+ packet_hexdump(data, size);
+ return;
+ }
+
+ if (vnd->evt_fixed) {
+ if (size != vnd->evt_size) {
+ print_text(COLOR_ERROR, "invalid packet size");
+ packet_hexdump(data, size);
+ return;
+ }
+ } else {
+ if (size < vnd->evt_size) {
+ print_text(COLOR_ERROR, "too short packet");
+ packet_hexdump(data, size);
+ return;
+ }
+ }
+
+ vnd->evt_func(data, size);
+ return;
+ }

+ subevent = *((const uint8_t *) data);
+ vnd = current_vendor_evt(subevent);
if (vnd) {
- const char *str = current_vendor_str();
+ const char *str = current_vendor_evt_str(subevent);

if (str) {
snprintf(vendor_str, sizeof(vendor_str),
@@ -11419,7 +11503,7 @@ void packet_hci_command(struct timeval *tv, struct ucred *cred, uint16_t index,
const struct vendor_ocf *vnd = current_vendor_ocf(ocf);

if (vnd) {
- const char *str = current_vendor_str();
+ const char *str = current_vendor_str(ocf);

if (str) {
snprintf(vendor_str, sizeof(vendor_str),
--
2.25.1


2021-04-21 02:42:23

by bluez.test.bot

[permalink] [raw]
Subject: RE: [BlueZ,v3] monitor: Fix the incorrect vendor name

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=470557

---Test result---

Test Summary:
CheckPatch FAIL 0.44 seconds
GitLint PASS 0.11 seconds
Prep - Setup ELL PASS 47.24 seconds
Build - Prep PASS 0.14 seconds
Build - Configure PASS 8.17 seconds
Build - Make PASS 204.14 seconds
Make Check PASS 8.95 seconds
Make Dist PASS 12.26 seconds
Make Dist - Configure PASS 4.99 seconds
Make Dist - Make PASS 84.00 seconds
Build w/ext ELL - Configure PASS 8.27 seconds
Build w/ext ELL - Make PASS 195.15 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
monitor: Fix the incorrect vendor name
WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#27: FILE: monitor/msft.h:31:
+} __attribute__ ((packed));

WARNING:BRACES: braces {} are not necessary for single statement blocks
#189: FILE: monitor/packet.c:9448:
+ if (size > prefix_len && !memcmp(data, prefix, prefix_len)) {
+ return msft_vendor_evt();
+ }

- total: 0 errors, 2 warnings, 240 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] monitor: Fix the incorrect vendor name" has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - PASS
Desc: Run gitlint with rule in .gitlint

##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL

##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build

##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree

##############################
Test: Build - Make - PASS
Desc: Build the BlueZ source tree

##############################
Test: Make Check - PASS
Desc: Run 'make check'

##############################
Test: Make Dist - PASS
Desc: Run 'make dist' and build the distribution tarball

##############################
Test: Make Dist - Configure - PASS
Desc: Configure the source from distribution tarball

##############################
Test: Make Dist - Make - PASS
Desc: Build the source from distribution tarball

##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration

##############################
Test: Build w/ext ELL - Make - PASS
Desc: Build BlueZ source with '--enable-external-ell' configuration



---
Regards,
Linux Bluetooth