tests: Bluetooth: Tester: BTP MTU handling improvements #84301
Conversation
zephyrbot
added
area: Bluetooth
area: Bluetooth Qualification
tests/bluetooth/tester/src/btp.c
Outdated
| struct btp_hdr hdr; | ||
| }; | ||
| uint8_t data[BTP_MTU]; /* includes btp header */ | ||
| uint8_t rsp[BTP_MTU - sizeof(struct btp_hdr)]; |
There was a problem hiding this comment.
Consider defining a BTP_RSP_MTU or something.
There was a problem hiding this comment.
RSP has same MTU, it is just that we don't have (need) header in response buffer (since data from command header is used)
| uint8_t status; | ||
| uint16_t rsp_len = 0; | ||
| uint16_t len; | ||
|
|
||
| cmd = k_fifo_get(&cmds_queue, K_FOREVER); | ||
| hdr = (struct btp_hdr *)cmd->data; |
There was a problem hiding this comment.
Should we have an
| hdr = (struct btp_hdr *)cmd->data; | |
| hdr = (struct btp_hdr *)cmd->data; | |
| __ASSERT_NO_MSG(hdr->len <= BTP_DATA_MAX_SIZE); |
There was a problem hiding this comment.
actually, I'm not sure if this should be assert as this means we assert on invalid command, I'll make this runtime check and simply return error (like we do for unknown command
|
The commit message doesn't really make sense anymore since |
This may cause some compiler warnings due to flexible array being used in btp_hdr. Simply avoid it and do explicit pointer type cast where needed. Signed-off-by: Szymon Janc <[email protected]>
sjanc
changed the title
Make sure MTU is validated to avoid reading pass command buffer. Also make more explicit check in response length validation. Signed-off-by: Szymon Janc <[email protected]>
| if (len > BTP_DATA_MAX_SIZE) { | ||
| status = BTP_STATUS_FAILED; | ||
| } else if (btp->index != hdr->index) { |
There was a problem hiding this comment.
Hmm, now I wonder if it's really need as we do have } else if ((btp->expect_len >= 0) && (btp->expect_len != len)) { which I guess would already catch it?
This is not allowed by C11 and may cause issues if GNU extensions are disabled.