Re: [PATCH v4] bluetooth: hci_qca: Fix QCA6390 memdump failure

[Marcel Holtmann]

Hi Zijun,

> QCA6390 memdump VSE sometimes come to bluetooth driver
> with wrong sequence number as illustrated as follows:
> frame # in dec: frame data in hex
> 1396: ff fd 01 08 74 05 00 37 8f 14
> 1397: ff fd 01 08 75 05 00 ff bf 38
> 1414: ff fd 01 08 86 05 00 fb 5e 4b
> 1399: ff fd 01 08 77 05 00 f3 44 0a
> 1400: ff fd 01 08 78 05 00 ca f7 41
> it is mistook for controller missing packets, so results
> in page fault after overwriting memdump buffer allocated.
> 
> Fixed by ignoring QCA6390 sequence number check and
> checking buffer space before writing.
> 
> Signed-off-by: Zijun Hu <zijuhu@xxxxxxxxxxxxxx>
> Tested-by: Zijun Hu <zijuhu@xxxxxxxxxxxxxx>
> ---
> Changes in v4:
> - add a piece of code comments
> Changes in v3:
> - correct coding style
> Changes in v2:
> - rename a local variable from @temp to @rx_size
> 
> drivers/bluetooth/hci_qca.c | 49 ++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 42 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index e4a6823..4acac13 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -114,6 +114,7 @@ struct qca_memdump_data {
> 	char *memdump_buf_tail;
> 	u32 current_seq_no;
> 	u32 received_dump;
> +	u32 ram_dump_size;
> };
> 
> struct qca_memdump_event_hdr {
> @@ -976,6 +977,8 @@ static void qca_controller_memdump(struct work_struct *work)
> 	char nullBuff[QCA_DUMP_PACKET_SIZE] = { 0 };
> 	u16 seq_no;
> 	u32 dump_size;
> +	u32 rx_size;
> +	enum qca_btsoc_type soc_type = qca_soc_type(hu);
> 
> 	while ((skb = skb_dequeue(&qca->rx_memdump_q))) {
> 
> @@ -1029,6 +1032,7 @@ static void qca_controller_memdump(struct work_struct *work)
> 
> 			skb_pull(skb, sizeof(dump_size));
> 			memdump_buf = vmalloc(dump_size);
> +			qca_memdump->ram_dump_size = dump_size;
> 			qca_memdump->memdump_buf_head = memdump_buf;
> 			qca_memdump->memdump_buf_tail = memdump_buf;
> 		}
> @@ -1051,26 +1055,57 @@ static void qca_controller_memdump(struct work_struct *work)
> 		 * the controller. In such cases let us store the dummy
> 		 * packets in the buffer.
> 		 */
> +		/* For QCA6390, controller does not lost packets but
> +		 * sequence number field of packat sometimes has error
> +		 * bits, so skip this checking for missing packet.
> +		 */
> 		while ((seq_no > qca_memdump->current_seq_no + 1) &&
> +			(soc_type != QCA_QCA6390) &&
> 			seq_no != QCA_LAST_SEQUENCE_NUM) {
> 			bt_dev_err(hu->hdev, "QCA controller missed packet:%d",
> 				   qca_memdump->current_seq_no);
> +			rx_size = qca_memdump->received_dump;
> +			rx_size += QCA_DUMP_PACKET_SIZE;
> +			if (rx_size > qca_memdump->ram_dump_size) {
> +				bt_dev_err(hu->hdev,
> +						"QCA memdump received %d, no space for missed packet",
> +						qca_memdump->received_dump);

please use correct indentation according to the coding style.

Regards

Marcel