[PATCH] Bluetooth: hci_h5: implement CRC data integrity

[PATCH] Bluetooth: hci_h5: implement CRC data integrity

[Javier Nieto]

The UART-based H5 protocol supports CRC data integrity checks for
reliable packets. The host sets bit 5 in the configuration field of the
CONFIG link control message to indicate that CRC is supported. The
controller sets the same bit in the CONFIG RESPONSE message to indicate
that CRC may be used from then on.

Signed-off-by: Javier Nieto <jgnieto@cs.stanford.edu>
---

Tested on a MangoPi MQ-Pro with a Realtek RTL8723DS Bluetooth controller
using the tip of the bluetooth-next tree.

It would be nice to have this feature available for somewhat more reliable
communication over UART, especially if RTS/CTS is disabled, as this is the
primary benefit of the H5 protocol. Thanks!

---
 drivers/bluetooth/hci_h5.c | 42 ++++++++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index d0d4420c1a0f..7faafc62666b 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -7,6 +7,8 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/bitrev.h>
+#include <linux/crc-ccitt.h>
 #include <linux/errno.h>
 #include <linux/gpio/consumer.h>
 #include <linux/kernel.h>
@@ -58,6 +60,7 @@ enum {
 	H5_TX_ACK_REQ,		/* Pending ack to send */
 	H5_WAKEUP_DISABLE,	/* Device cannot wake host */
 	H5_HW_FLOW_CONTROL,	/* Use HW flow control */
+	H5_CRC,			/* Use CRC */
 };
 
 struct h5 {
@@ -141,8 +144,8 @@ static void h5_link_control(struct hci_uart *hu, const void *data, size_t len)
 
 static u8 h5_cfg_field(struct h5 *h5)
 {
-	/* Sliding window size (first 3 bits) */
-	return h5->tx_win & 0x07;
+	/* Sliding window size (first 3 bits) and CRC request (fifth bit). */
+	return (h5->tx_win & 0x07) | 0x10;
 }
 
 static void h5_timed_event(struct timer_list *t)
@@ -360,8 +363,10 @@ static void h5_handle_internal_rx(struct hci_uart *hu)
 		h5_link_control(hu, conf_rsp, 2);
 		h5_link_control(hu, conf_req, 3);
 	} else if (memcmp(data, conf_rsp, 2) == 0) {
-		if (H5_HDR_LEN(hdr) > 2)
+		if (H5_HDR_LEN(hdr) > 2) {
 			h5->tx_win = (data[2] & 0x07);
+			assign_bit(H5_CRC, &h5->flags, data[2] & 0x10);
+		}
 		BT_DBG("Three-wire init complete. tx_win %u", h5->tx_win);
 		h5->state = H5_ACTIVE;
 		hci_uart_init_ready(hu);
@@ -425,7 +430,24 @@ static void h5_complete_rx_pkt(struct hci_uart *hu)
 
 static int h5_rx_crc(struct hci_uart *hu, unsigned char c)
 {
-	h5_complete_rx_pkt(hu);
+	struct h5 *h5 = hu->priv;
+	const unsigned char *hdr = h5->rx_skb->data;
+	u16 crc;
+	__be16 crc_be;
+
+	crc = crc_ccitt(0xffff, hdr, 4 + H5_HDR_LEN(hdr));
+	crc = bitrev16(crc);
+
+	crc_be = cpu_to_be16(crc);
+
+	if (memcmp(&crc_be, hdr + 4 + H5_HDR_LEN(hdr), 2) != 0) {
+		bt_dev_err(hu->hdev, "Received packet with invalid CRC");
+		h5_reset_rx(h5);
+	} else {
+		/* Remove CRC bytes */
+		skb_trim(h5->rx_skb, 4 + H5_HDR_LEN(hdr));
+		h5_complete_rx_pkt(hu);
+	}
 
 	return 0;
 }
@@ -556,6 +578,7 @@ static void h5_reset_rx(struct h5 *h5)
 	h5->rx_func = h5_rx_delimiter;
 	h5->rx_pending = 0;
 	clear_bit(H5_RX_ESC, &h5->flags);
+	clear_bit(H5_CRC, &h5->flags);
 }
 
 static int h5_recv(struct hci_uart *hu, const void *data, int count)
@@ -686,6 +709,7 @@ static struct sk_buff *h5_prepare_pkt(struct hci_uart *hu, u8 pkt_type,
 	struct h5 *h5 = hu->priv;
 	struct sk_buff *nskb;
 	u8 hdr[4];
+	u16 crc;
 	int i;
 
 	if (!valid_packet_type(pkt_type)) {
@@ -713,6 +737,7 @@ static struct sk_buff *h5_prepare_pkt(struct hci_uart *hu, u8 pkt_type,
 	/* Reliable packet? */
 	if (pkt_type == HCI_ACLDATA_PKT || pkt_type == HCI_COMMAND_PKT) {
 		hdr[0] |= 1 << 7;
+		hdr[0] |= (test_bit(H5_CRC, &h5->flags) && 1) << 6;
 		hdr[0] |= h5->tx_seq;
 		h5->tx_seq = (h5->tx_seq + 1) % 8;
 	}
@@ -732,6 +757,15 @@ static struct sk_buff *h5_prepare_pkt(struct hci_uart *hu, u8 pkt_type,
 	for (i = 0; i < len; i++)
 		h5_slip_one_byte(nskb, data[i]);
 
+	if (H5_HDR_CRC(hdr)) {
+		crc = crc_ccitt(0xffff, hdr, 4);
+		crc = crc_ccitt(crc, data, len);
+		crc = bitrev16(crc);
+
+		h5_slip_one_byte(nskb, (crc >> 8) & 0xff);
+		h5_slip_one_byte(nskb, crc & 0xff);
+	}
+
 	h5_slip_delim(nskb);
 
 	return nskb;
-- 
2.43.0

RE: Bluetooth: hci_h5: implement CRC data integrity

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 6301 bytes --]

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

---Test result---

Test Summary:
CheckPatch                    PENDING   0.39 seconds
GitLint                       PENDING   0.21 seconds
SubjectPrefix                 PASS      0.12 seconds
BuildKernel                   PASS      23.96 seconds
CheckAllWarning               PASS      26.44 seconds
CheckSparse                   PASS      29.68 seconds
BuildKernel32                 PASS      24.10 seconds
TestRunnerSetup               FAIL      462.90 seconds
TestRunner_l2cap-tester       FAIL      0.14 seconds
TestRunner_iso-tester         FAIL      0.14 seconds
TestRunner_bnep-tester        FAIL      0.14 seconds
TestRunner_mgmt-tester        FAIL      0.14 seconds
TestRunner_rfcomm-tester      FAIL      0.15 seconds
TestRunner_sco-tester         FAIL      0.14 seconds
TestRunner_ioctl-tester       FAIL      0.14 seconds
TestRunner_mesh-tester        FAIL      0.14 seconds
TestRunner_smp-tester         FAIL      0.14 seconds
TestRunner_userchan-tester    FAIL      0.14 seconds
IncrementalBuild              PENDING   0.52 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: TestRunnerSetup - FAIL
Desc: Setup kernel and bluez for test-runner
Output:
Kernel: 
ld: vmlinux.o: in function `h5_rx_crc':
hci_h5.c:(.text+0xd938d9): undefined reference to `crc_ccitt'
ld: vmlinux.o: in function `h5_prepare_pkt':
hci_h5.c:(.text+0xd94798): undefined reference to `crc_ccitt'
ld: hci_h5.c:(.text+0xd947a6): undefined reference to `crc_ccitt'
make[2]: *** [scripts/Makefile.vmlinux:91: vmlinux.unstripped] Error 1
make[1]: *** [/github/workspace/src/src/Makefile:1236: vmlinux] Error 2
make: *** [Makefile:248: __sub-make] Error 2
##############################
Test: TestRunner_l2cap-tester - FAIL
Desc: Run l2cap-tester with test-runner
Output:

Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:

Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_bnep-tester - FAIL
Desc: Run bnep-tester with test-runner
Output:

Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:

Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_rfcomm-tester - FAIL
Desc: Run rfcomm-tester with test-runner
Output:

Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_sco-tester - FAIL
Desc: Run sco-tester with test-runner
Output:

Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_ioctl-tester - FAIL
Desc: Run ioctl-tester with test-runner
Output:

Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:

Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_smp-tester - FAIL
Desc: Run smp-tester with test-runner
Output:

Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_userchan-tester - FAIL
Desc: Run userchan-tester with test-runner
Output:

Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

Re: [PATCH] Bluetooth: hci_h5: implement CRC data integrity

[Paul Menzel]

Dear Javier,


Thank you very much for the patch. Great work!

Am 27.08.25 um 06:32 schrieb Javier Nieto:
> The UART-based H5 protocol supports CRC data integrity checks for
> reliable packets. The host sets bit 5 in the configuration field of the
> CONFIG link control message to indicate that CRC is supported. The
> controller sets the same bit in the CONFIG RESPONSE message to indicate
> that CRC may be used from then on.
> 
> Signed-off-by: Javier Nieto <jgnieto@cs.stanford.edu>
> ---
> 
> Tested on a MangoPi MQ-Pro with a Realtek RTL8723DS Bluetooth controller
> using the tip of the bluetooth-next tree.

Any btmon trace?

I’d add the above to the proper commit message.

> It would be nice to have this feature available for somewhat more reliable
> communication over UART, especially if RTS/CTS is disabled, as this is the
> primary benefit of the H5 protocol. Thanks!
> 
> ---
>   drivers/bluetooth/hci_h5.c | 42 ++++++++++++++++++++++++++++++++++----
>   1 file changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
> index d0d4420c1a0f..7faafc62666b 100644
> --- a/drivers/bluetooth/hci_h5.c
> +++ b/drivers/bluetooth/hci_h5.c
> @@ -7,6 +7,8 @@
>    */
>   
>   #include <linux/acpi.h>
> +#include <linux/bitrev.h>
> +#include <linux/crc-ccitt.h>
>   #include <linux/errno.h>
>   #include <linux/gpio/consumer.h>
>   #include <linux/kernel.h>
> @@ -58,6 +60,7 @@ enum {
>   	H5_TX_ACK_REQ,		/* Pending ack to send */
>   	H5_WAKEUP_DISABLE,	/* Device cannot wake host */
>   	H5_HW_FLOW_CONTROL,	/* Use HW flow control */
> +	H5_CRC,			/* Use CRC */
>   };
>   
>   struct h5 {
> @@ -141,8 +144,8 @@ static void h5_link_control(struct hci_uart *hu, const void *data, size_t len)
>   
>   static u8 h5_cfg_field(struct h5 *h5)
>   {
> -	/* Sliding window size (first 3 bits) */
> -	return h5->tx_win & 0x07;
> +	/* Sliding window size (first 3 bits) and CRC request (fifth bit). */
> +	return (h5->tx_win & 0x07) | 0x10;

Could a macro be defined for the CRC request bit?

>   }
>   
>   static void h5_timed_event(struct timer_list *t)
> @@ -360,8 +363,10 @@ static void h5_handle_internal_rx(struct hci_uart *hu)
>   		h5_link_control(hu, conf_rsp, 2);
>   		h5_link_control(hu, conf_req, 3);
>   	} else if (memcmp(data, conf_rsp, 2) == 0) {
> -		if (H5_HDR_LEN(hdr) > 2)
> +		if (H5_HDR_LEN(hdr) > 2) {
>   			h5->tx_win = (data[2] & 0x07);
> +			assign_bit(H5_CRC, &h5->flags, data[2] & 0x10);
> +		}
>   		BT_DBG("Three-wire init complete. tx_win %u", h5->tx_win);
>   		h5->state = H5_ACTIVE;
>   		hci_uart_init_ready(hu);
> @@ -425,7 +430,24 @@ static void h5_complete_rx_pkt(struct hci_uart *hu)
>   
>   static int h5_rx_crc(struct hci_uart *hu, unsigned char c)
>   {
> -	h5_complete_rx_pkt(hu);
> +	struct h5 *h5 = hu->priv;
> +	const unsigned char *hdr = h5->rx_skb->data;
> +	u16 crc;
> +	__be16 crc_be;
> +
> +	crc = crc_ccitt(0xffff, hdr, 4 + H5_HDR_LEN(hdr));
> +	crc = bitrev16(crc);
> +
> +	crc_be = cpu_to_be16(crc);
> +
> +	if (memcmp(&crc_be, hdr + 4 + H5_HDR_LEN(hdr), 2) != 0) {
> +		bt_dev_err(hu->hdev, "Received packet with invalid CRC");
> +		h5_reset_rx(h5);
> +	} else {
> +		/* Remove CRC bytes */
> +		skb_trim(h5->rx_skb, 4 + H5_HDR_LEN(hdr));
> +		h5_complete_rx_pkt(hu);
> +	}
>   
>   	return 0;
>   }
> @@ -556,6 +578,7 @@ static void h5_reset_rx(struct h5 *h5)
>   	h5->rx_func = h5_rx_delimiter;
>   	h5->rx_pending = 0;
>   	clear_bit(H5_RX_ESC, &h5->flags);
> +	clear_bit(H5_CRC, &h5->flags);
>   }
>   
>   static int h5_recv(struct hci_uart *hu, const void *data, int count)
> @@ -686,6 +709,7 @@ static struct sk_buff *h5_prepare_pkt(struct hci_uart *hu, u8 pkt_type,
>   	struct h5 *h5 = hu->priv;
>   	struct sk_buff *nskb;
>   	u8 hdr[4];
> +	u16 crc;
>   	int i;
>   
>   	if (!valid_packet_type(pkt_type)) {
> @@ -713,6 +737,7 @@ static struct sk_buff *h5_prepare_pkt(struct hci_uart *hu, u8 pkt_type,
>   	/* Reliable packet? */
>   	if (pkt_type == HCI_ACLDATA_PKT || pkt_type == HCI_COMMAND_PKT) {
>   		hdr[0] |= 1 << 7;
> +		hdr[0] |= (test_bit(H5_CRC, &h5->flags) && 1) << 6;
>   		hdr[0] |= h5->tx_seq;
>   		h5->tx_seq = (h5->tx_seq + 1) % 8;
>   	}
> @@ -732,6 +757,15 @@ static struct sk_buff *h5_prepare_pkt(struct hci_uart *hu, u8 pkt_type,
>   	for (i = 0; i < len; i++)
>   		h5_slip_one_byte(nskb, data[i]);
>   
> +	if (H5_HDR_CRC(hdr)) {
> +		crc = crc_ccitt(0xffff, hdr, 4);
> +		crc = crc_ccitt(crc, data, len);
> +		crc = bitrev16(crc);
> +
> +		h5_slip_one_byte(nskb, (crc >> 8) & 0xff);
> +		h5_slip_one_byte(nskb, crc & 0xff);
> +	}
> +
>   	h5_slip_delim(nskb);
>   
>   	return nskb;

The diff looks good. Feel free to carry:

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

Re: [PATCH] Bluetooth: hci_h5: implement CRC data integrity

[Javier Nieto]

Dear Paul,

Thanks for the review!

On Wed, Aug 27, 2025 at 12:56:50PM +0200, Paul Menzel wrote:

> Any btmon trace?

The presence of CRC is limited to the H5 layer, so it is not visible on
btmon. However, I did advertise and connect to a few devices while
running btmon and everything worked and looked as normal. I also ensured
that CRC was being used by adding temporary debugging prints.

> I´d add the above to the proper commit message.

Should I resubmit the patch as v2?

> >   static u8 h5_cfg_field(struct h5 *h5)
> >   {
> > -	/* Sliding window size (first 3 bits) */
> > -	return h5->tx_win & 0x07;
> > +	/* Sliding window size (first 3 bits) and CRC request (fifth bit). */
> > +	return (h5->tx_win & 0x07) | 0x10;
> 
> Could a macro be defined for the CRC request bit?

I thought about this, but decided against it since 0x10 is only used
here and in one other place. Also, the existing code does not define a
macro for the window size bits 0x07. I am not opposed to adding it if
someone feels strongly about it though.

> The diff looks good. Feel free to carry:
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> 
> 
> Kind regards,
> 
> Paul

I see that my patch fails a few test cases because it fails to link
crc-ccitt. Do you know whether this is a problem with my patch or the
test environment and where the code for the tests is found?

Thanks again for your feedback.

Javier