Skip to content

Commit

Permalink
Bluetooth: AVRCP: fix bitfield issue.
Browse files Browse the repository at this point in the history 
The bit order can be incorrect when use bit field definition.

Signed-off-by: Zihao Gao <[email protected]>
  • Loading branch information
Zihao Gao committed Oct 15, 2024
1 parent 8cb6634 commit d0cfd32
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 44 deletions.
23 changes: 14 additions & 9 deletions subsys/bluetooth/host/classic/avctp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ static int avctp_l2cap_recv(struct bt_l2cap_chan *chan, struct net_buf *buf)
struct net_buf *rsp;
struct bt_avctp *session = AVCTP_CHAN(chan);
struct bt_avctp_header *hdr = (void *)buf->data;
uint8_t tid = BT_AVCTP_HDR_GET_TRANSACTION_LABLE(hdr);
bt_avctp_cr_t cr = BT_AVCTP_HDR_GET_CR(hdr);

if (buf->len < sizeof(*hdr)) {
LOG_ERR("invalid AVCTP header received");
Expand All @@ -86,9 +88,10 @@ static int avctp_l2cap_recv(struct bt_l2cap_chan *chan, struct net_buf *buf)
#endif
default:
LOG_ERR("unsupported AVCTP PID received: 0x%04x", sys_be16_to_cpu(hdr->pid));
if (hdr->cr == BT_AVCTP_CMD) {
rsp = bt_avctp_create_pdu(session, BT_AVCTP_RESPONSE, BT_AVCTP_IPID_INVALID,
&hdr->tid, hdr->pid);
if (cr == BT_AVCTP_CMD) {
rsp = bt_avctp_create_pdu(session, BT_AVCTP_RESPONSE,
BT_AVCTP_PKT_TYPE_SINGLE, BT_AVCTP_IPID_INVALID,
&tid, hdr->pid);
if (!rsp) {
return -ENOMEM;
}
Expand Down Expand Up @@ -132,7 +135,8 @@ int bt_avctp_disconnect(struct bt_avctp *session)
}

struct net_buf *bt_avctp_create_pdu(struct bt_avctp *session, bt_avctp_cr_t cr,
bt_avctp_ipid_t ipid, uint8_t *tid, uint16_t pid)
bt_avctp_pkt_type_t pkt_type, bt_avctp_ipid_t ipid,
uint8_t *tid, uint16_t pid)
{
struct net_buf *buf;
struct bt_avctp_header *hdr;
Expand All @@ -146,17 +150,18 @@ struct net_buf *bt_avctp_create_pdu(struct bt_avctp *session, bt_avctp_cr_t cr,
}

hdr = net_buf_add(buf, sizeof(*hdr));
hdr->cr = cr;
hdr->ipid = ipid;
hdr->pkt_type = BT_AVCTP_PKT_TYPE_SINGLE;
hdr->tid = *tid;
BT_AVCTP_HDR_SET_TRANSACTION_LABLE(hdr, *tid);
BT_AVCTP_HDR_SET_PACKET_TYPE(hdr, pkt_type);
BT_AVCTP_HDR_SET_CR(hdr, cr);
BT_AVCTP_HDR_SET_IPID(hdr, ipid);
hdr->pid = pid;

if (cr == BT_AVCTP_CMD) {
*tid = (*tid + 1) & 0x0F; /* Incremented by one */
}

LOG_DBG("cr:0x%X, tid:0x%02X", hdr->cr, hdr->tid);
LOG_DBG("cr:0x%lX, tid:0x%02lX", BT_AVCTP_HDR_GET_CR(hdr),
BT_AVCTP_HDR_GET_TRANSACTION_LABLE(hdr));
return buf;
}

Expand Down
51 changes: 45 additions & 6 deletions subsys/bluetooth/host/classic/avctp_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,54 @@ typedef enum __packed {

typedef enum __packed {
BT_AVCTP_PKT_TYPE_SINGLE = 0b00,
BT_AVCTP_PKT_TYPE_START = 0b01,
BT_AVCTP_PKT_TYPE_CONTINUE = 0b10,
BT_AVCTP_PKT_TYPE_END = 0b11,
} bt_avctp_pkt_type_t;

struct bt_avctp_header {
uint8_t ipid: 1; /* Invalid Profile Identifier (1), otherwise zero (0) */
uint8_t cr: 1; /* Command(0) or Respone(1) */
uint8_t pkt_type: 2; /* Set to zero (00) for single L2CAP packet */
uint8_t tid: 4; /* Transaction label */
uint16_t pid; /* Profile Identifier */
uint8_t byte0; /** [7:4]: Transaction label, [3:2]: Packet_type, [1]: C/R, [0]: IPID */
uint16_t pid; /** Profile Identifier */
} __packed;

/** Transaction label provided by the application and is replicated by the sender of the message in
* each packet of the sequence. It isused at the receiver side to identify packets that belong to
* the same message.
*/
#define BT_AVCTP_HDR_GET_TRANSACTION_LABLE(hdr) FIELD_GET(GENMASK(7, 4), ((hdr)->byte0))
/** Set to zero (00) to indicate that the command/response message is transmitted in a single L2CAP
* packet. Alternatively, set to (01) for start, (10) for continue, or (11) for end packet.
*/
#define BT_AVCTP_HDR_GET_PACKET_TYPE(hdr) FIELD_GET(GENMASK(3, 2), ((hdr)->byte0))
/** Indicates whether the messageconveys a command frame (0) or a response frame (1). */
#define BT_AVCTP_HDR_GET_CR(hdr) FIELD_GET(BIT(1), ((hdr)->byte0))
/** The IPID bit is set in a response message to indicate an invalid Profile Identifier received in
* the command message of the same transaction; otherwise this bit is set to zero. In command
* messages this bit is set to zero. This field is only present in the start packet of the message.
*/
#define BT_AVCTP_HDR_GET_IPID(hdr) FIELD_GET(BIT(0), ((hdr)->byte0))

/** Transaction label provided by the application and is replicated by the sender of the message in
* each packet of the sequence. It isused at the receiver side to identify packets that belong to
* the same message.
*/
#define BT_AVCTP_HDR_SET_TRANSACTION_LABLE(hdr, tl) \
(hdr)->byte0 = (((hdr)->byte0) & ~GENMASK(7, 4)) | FIELD_PREP(GENMASK(7, 4), (tl))
/** Set to zero (00) to indicate that the command/response message is transmitted in a single L2CAP
* packet. Alternatively, set to (01) for start, (10) for continue, or (11) for end packet.
*/
#define BT_AVCTP_HDR_SET_PACKET_TYPE(hdr, packet_type) \
(hdr)->byte0 = (((hdr)->byte0) & ~GENMASK(3, 2)) | FIELD_PREP(GENMASK(3, 2), (packet_type))
/** Indicates whether the messageconveys a command frame (0) or a response frame (1). */
#define BT_AVCTP_HDR_SET_CR(hdr, cr) \
(hdr)->byte0 = (((hdr)->byte0) & ~BIT(1)) | FIELD_PREP(BIT(1), (cr))
/** The IPID bit is set in a response message to indicate an invalid Profile Identifier received in
* the command message of the same transaction; otherwise this bit is set to zero. In command
* messages this bit is set to zero. This field is only present in the start packet of the message.
*/
#define BT_AVCTP_HDR_SET_IPID(hdr, ipid) \
(hdr)->byte0 = (((hdr)->byte0) & ~BIT(0)) | FIELD_PREP(BIT(0), (ipid))

struct bt_avctp;

struct bt_avctp_ops_cb {
Expand Down Expand Up @@ -64,7 +102,8 @@ int bt_avctp_disconnect(struct bt_avctp *session);

/* Create AVCTP PDU */
struct net_buf *bt_avctp_create_pdu(struct bt_avctp *session, bt_avctp_cr_t cr,
bt_avctp_ipid_t ipid, uint8_t *tid, uint16_t pid);
bt_avctp_pkt_type_t pkt_type, bt_avctp_ipid_t ipid,
uint8_t *tid, uint16_t pid);

/* Send AVCTP PDU */
int bt_avctp_send(struct bt_avctp *session, struct net_buf *buf);
61 changes: 37 additions & 24 deletions subsys/bluetooth/host/classic/avrcp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,26 +246,36 @@ static int avrcp_recv(struct bt_avctp *session, struct net_buf *buf)
struct bt_avrcp *avrcp = AVRCP_AVCTP(session);
struct bt_avctp_header *avctp_hdr;
struct bt_avrcp_header *avrcp_hdr;
uint8_t tid;
bt_avctp_cr_t cr;
bt_avrcp_ctype_t ctype;
bt_avrcp_subunit_id_t subunit_id;
bt_avrcp_subunit_type_t subunit_type;

avctp_hdr = (void *)buf->data;
net_buf_pull(buf, sizeof(*avctp_hdr));
avrcp_hdr = (void *)buf->data;

tid = BT_AVCTP_HDR_GET_TRANSACTION_LABLE(avctp_hdr);
cr = BT_AVCTP_HDR_GET_CR(avctp_hdr);
ctype = BT_AVRCP_HDR_GET_CTYPE(avrcp_hdr);
subunit_id = BT_AVRCP_HDR_GET_SUBUNIT_ID(avrcp_hdr);
subunit_type = BT_AVRCP_HDR_GET_SUBUNIT_TYPE(avrcp_hdr);

if (avctp_hdr->pid != sys_cpu_to_be16(BT_SDP_AV_REMOTE_SVCLASS)) {
return -EINVAL; /* Ignore other profile */
}

LOG_DBG("AVRCP msg received, cr:0x%X, tid:0x%X, ctype: 0x%X, opc:0x%02X,", avctp_hdr->cr,
avctp_hdr->tid, avrcp_hdr->ctype, avrcp_hdr->opcode);
if (avctp_hdr->cr == BT_AVCTP_RESPONSE) {
LOG_DBG("AVRCP msg received, cr:0x%X, tid:0x%X, ctype: 0x%X, opc:0x%02X,", cr, tid, ctype,
avrcp_hdr->opcode);
if (cr == BT_AVCTP_RESPONSE) {
if (avrcp_hdr->opcode == BT_AVRCP_OPC_VENDOR_DEPENDENT &&
avrcp_hdr->ctype == BT_AVRCP_CTYPE_CHANGED) {
ctype == BT_AVRCP_CTYPE_CHANGED) {
/* Status changed notifiation, do not reset timer */
} else if (avrcp_hdr->opcode == BT_AVRCP_OPC_PASS_THROUGH) {
/* No max response time for pass through commands */
} else if (avrcp->req.tid != avctp_hdr->tid ||
avrcp->req.subunit != avrcp_hdr->subunit_id ||
avrcp->req.opcode != avrcp_hdr->opcode) {
} else if (tid != avrcp->req.tid || subunit_id != avrcp->req.subunit ||
avrcp_hdr->opcode != avrcp->req.opcode) {
LOG_WRN("unexpected AVRCP response, expected tid:0x%X, subunit:0x%X, "
"opc:0x%02X",
avrcp->req.tid, avrcp->req.subunit, avrcp->req.opcode);
Expand Down Expand Up @@ -370,7 +380,8 @@ static struct net_buf *avrcp_create_pdu(struct bt_avrcp *avrcp, bt_avctp_cr_t cr
{
struct net_buf *buf;

buf = bt_avctp_create_pdu(&(avrcp->session), cr, BT_AVCTP_IPID_NONE, &avrcp->local_tid,
buf = bt_avctp_create_pdu(&(avrcp->session), cr, BT_AVCTP_PKT_TYPE_SINGLE,
BT_AVCTP_IPID_NONE, &avrcp->local_tid,
sys_cpu_to_be16(BT_SDP_AV_REMOTE_SVCLASS));

return buf;
Expand All @@ -388,10 +399,10 @@ static struct net_buf *avrcp_create_unit_pdu(struct bt_avrcp *avrcp, bt_avctp_cr

cmd = net_buf_add(buf, sizeof(*cmd));
memset(cmd, 0, sizeof(*cmd));
cmd->hdr.ctype =
(cr == BT_AVCTP_CMD) ? BT_AVRCP_CTYPE_STATUS : BT_AVRCP_CTYPE_IMPLEMENTED_STABLE;
cmd->hdr.subunit_id = BT_AVRCP_SUBUNIT_ID_IGNORE;
cmd->hdr.subunit_type = BT_AVRCP_SUBUNIT_TYPE_UNIT;
BT_AVRCP_HDR_SET_CTYPE(&cmd->hdr, cr == BT_AVCTP_CMD ? BT_AVRCP_CTYPE_STATUS
: BT_AVRCP_CTYPE_IMPLEMENTED_STABLE);
BT_AVRCP_HDR_SET_SUBUNIT_ID(&cmd->hdr, BT_AVRCP_SUBUNIT_ID_IGNORE);
BT_AVRCP_HDR_SET_SUBUNIT_TYPE(&cmd->hdr, BT_AVRCP_SUBUNIT_TYPE_UNIT);
cmd->hdr.opcode = BT_AVRCP_OPC_UNIT_INFO;

return buf;
Expand All @@ -400,23 +411,25 @@ static struct net_buf *avrcp_create_unit_pdu(struct bt_avrcp *avrcp, bt_avctp_cr
static int avrcp_send(struct bt_avrcp *avrcp, struct net_buf *buf)
{
int err;
struct bt_avctp_header avctp_hdr;
struct bt_avrcp_header avrcp_hdr;

memcpy(&avctp_hdr, buf->data, sizeof(avctp_hdr));
memcpy(&avrcp_hdr, buf->data + sizeof(avctp_hdr), sizeof(avrcp_hdr));

LOG_DBG("AVRCP send cr:0x%X, tid:0x%X, ctype: 0x%X, opc:0x%02X\n", avctp_hdr.cr,
avctp_hdr.tid, avrcp_hdr.ctype, avrcp_hdr.opcode);
struct bt_avctp_header *avctp_hdr = (struct bt_avctp_header *)(buf->data);
struct bt_avrcp_header *avrcp_hdr =
(struct bt_avrcp_header *)(buf->data + sizeof(*avctp_hdr));
uint8_t tid = BT_AVCTP_HDR_GET_TRANSACTION_LABLE(avctp_hdr);
bt_avctp_cr_t cr = BT_AVCTP_HDR_GET_CR(avctp_hdr);
bt_avrcp_ctype_t ctype = BT_AVRCP_HDR_GET_CTYPE(avrcp_hdr);
bt_avrcp_subunit_type_t subunit_id = BT_AVRCP_HDR_GET_SUBUNIT_ID(avrcp_hdr);

LOG_DBG("AVRCP send cr:0x%X, tid:0x%X, ctype: 0x%X, opc:0x%02X\n", cr, tid, ctype,
avrcp_hdr->opcode);
err = bt_avctp_send(&(avrcp->session), buf);
if (err < 0) {
return err;
}

if (avctp_hdr.cr == BT_AVCTP_CMD && avrcp_hdr.opcode != BT_AVRCP_OPC_PASS_THROUGH) {
avrcp->req.tid = avctp_hdr.tid;
avrcp->req.subunit = avrcp_hdr.subunit_id;
avrcp->req.opcode = avrcp_hdr.opcode;
if (cr == BT_AVCTP_CMD && avrcp_hdr->opcode != BT_AVRCP_OPC_PASS_THROUGH) {
avrcp->req.tid = tid;
avrcp->req.subunit = subunit_id;
avrcp->req.opcode = avrcp_hdr->opcode;

k_work_reschedule(&avrcp->timeout_work, AVRCP_TIMEOUT);
}
Expand Down
41 changes: 36 additions & 5 deletions subsys/bluetooth/host/classic/avrcp_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,44 @@ struct bt_avrcp_req {
};

struct bt_avrcp_header {
uint8_t ctype: 4; /* Command type codes */
uint8_t rfa: 4; /* Zero according to AV/C command frame */
uint8_t subunit_id: 3; /* Zero (0x0) or Ignore (0x7) according to AVRCP */
uint8_t subunit_type: 5; /* Unit (0x1F) or Panel (0x9) according to AVRCP */
uint8_t opcode; /* Unit Info, Subunit Info, Vendor Dependent, or Pass Through */
uint8_t byte0; /** [7:4]: RFA, [3:0]: Ctype */
uint8_t byte1; /** [7:3]: Subunit_type, [2:0]: Subunit_ID */
uint8_t opcode; /** Unit Info, Subunit Info, Vendor Dependent, or Pass Through */
} __packed;

/** The 4-bit command type or the 4-bit response code. */
#define BT_AVRCP_HDR_GET_CTYPE(hdr) FIELD_GET(GENMASK(3, 0), ((hdr)->byte0))
/** Taken together, the subunit_type and subunit_ID fields define the command recipient’s address
* within the target. These fields enable the target to determine whether the command is
* addressed to the target unit, or to a specific subunit within the target. The values in these
* fields remain unchanged in the response frame.
*/
#define BT_AVRCP_HDR_GET_SUBUNIT_ID(hdr) FIELD_GET(GENMASK(2, 0), ((hdr)->byte1))
/** Taken together, the subunit_type and subunit_ID fields define the command recipient’s address
* within the target. These fields enable the target to determine whether the command is
* addressed to the target unit, or to a specific subunit within the target. The values in these
* fields remain unchanged in the response frame.
*/
#define BT_AVRCP_HDR_GET_SUBUNIT_TYPE(hdr) FIELD_GET(GENMASK(7, 3), ((hdr)->byte1))

/** The 4-bit command type or the 4-bit response code. */
#define BT_AVRCP_HDR_SET_CTYPE(hdr, ctype) \
(hdr)->byte0 = (((hdr)->byte0) & ~GENMASK(3, 0)) | FIELD_PREP(GENMASK(3, 0), (ctype))
/** Taken together, the subunit_type and subunit_ID fields define the command recipient’s address
* within the target. These fields enable the target to determine whether the command is
* addressed to the target unit, or to a specific subunit within the target. The values in these
* fields remain unchanged in the response frame.
*/
#define BT_AVRCP_HDR_SET_SUBUNIT_ID(hdr, subunit_id) \
(hdr)->byte1 = (((hdr)->byte1) & ~GENMASK(2, 0)) | FIELD_PREP(GENMASK(2, 0), (subunit_id))
/** Taken together, the subunit_type and subunit_ID fields define the command recipient’s address
* within the target. These fields enable the target to determine whether the command is
* addressed to the target unit, or to a specific subunit within the target. The values in these
* fields remain unchanged in the response frame.
*/
#define BT_AVRCP_HDR_SET_SUBUNIT_TYPE(hdr, subunit_type) \
(hdr)->byte1 = (((hdr)->byte1) & ~GENMASK(7, 3)) | FIELD_PREP(GENMASK(7, 3), (subunit_type))

struct bt_avrcp_unit_info_cmd {
struct bt_avrcp_header hdr;
uint8_t data[0];
Expand Down

0 comments on commit d0cfd32

Please sign in to comment.