Skip to contentSkip to conversation

soc: qcom: qmi: fix a buffer sizing bug

PATCH In review Alex Elder 2 participants 2 messages Apr 27, 2018 – May 4, 2018

Subsystems (2)
lore
Share & tools
lore

Alex Elder

author
#0 lore raw

PATCH 1/1 soc: qcom: qmi: fix a buffer sizing bug

In qmi_handle_init(), a buffer is allocated for to hold messages received through the handle's socket. Any "normal" messages (expected by the caller) will have a header prepended, so the buffer size is adjusted to accomodate that.

The buffer must also be of sufficient size to receive control messages, so the size is increased if necessary to ensure these will fit.

Unfortunately the calculation is done wrong, making it possible for the calculated buffer size to be too small to hold a "normal" message. Specifically, if:

recv_buf_size > sizeof(struct qrtr_ctrl_pkt) - sizeof(struct qmi_header)
AND
recv_buf_size < sizeof(struct qrtr_ctrl_pkt)

the current logic will use sizeof(struct qrtr_ctrl_pkt) as the receive buffer size, which is not enough to hold the maximum "normal" message plus its header. Currently this problem occurs for (13 < recv_buf_size < 20).

This patch corrects this.

---
 drivers/soc/qcom/qmi_interface.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/soc/qcom/qmi_interface.c +3 -2
index 321982277697..938ca41c56cd 100644
@@ -639,10 +639,11 @@ int qmi_handle_init(struct qmi_handle *qmi, size_t recv_buf_size,
 	if (ops)
 		qmi->ops = *ops;
 
+	/* Make room for the header */
+	recv_buf_size += sizeof(struct qmi_header);
+	/* Must also be sufficient to hold a control packet */
 	if (recv_buf_size < sizeof(struct qrtr_ctrl_pkt))
 		recv_buf_size = sizeof(struct qrtr_ctrl_pkt);
-	else
-		recv_buf_size += sizeof(struct qmi_header);
 
 	qmi->recv_buf_size = recv_buf_size;
 	qmi->recv_buf = kzalloc(recv_buf_size, GFP_KERNEL);
Signature · 1 line
2.14.1
Reviewed-by on v1 1/1
#1 lore raw
Quoted 17 lines · Alex Elder

In qmi_handle_init(), a buffer is allocated for to hold messages received through the handle's socket. Any "normal" messages (expected by the caller) will have a header prepended, so the buffer size is adjusted to accomodate that.

The buffer must also be of sufficient size to receive control messages, so the size is increased if necessary to ensure these will fit.

Unfortunately the calculation is done wrong, making it possible for the calculated buffer size to be too small to hold a "normal" message. Specifically, if:

recv_buf_size > sizeof(struct qrtr_ctrl_pkt) - sizeof(struct qmi_header)
AND
recv_buf_size < sizeof(struct qrtr_ctrl_pkt)

Quoted 8 lines

the current logic will use sizeof(struct qrtr_ctrl_pkt) as the receive buffer size, which is not enough to hold the maximum "normal" message plus its header. Currently this problem occurs for (13 < recv_buf_size < 20).

This patch corrects this.

Good catch, thanks!

Regards,
Bjorn

Quoted 25 lines
---
 drivers/soc/qcom/qmi_interface.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/soc/qcom/qmi_interface.c +3 -2
index 321982277697..938ca41c56cd 100644
@@ -639,10 +639,11 @@ int qmi_handle_init(struct qmi_handle *qmi, size_t recv_buf_size,
 	if (ops)
 		qmi->ops = *ops;
 
+	/* Make room for the header */
+	recv_buf_size += sizeof(struct qmi_header);
+	/* Must also be sufficient to hold a control packet */
 	if (recv_buf_size < sizeof(struct qrtr_ctrl_pkt))
 		recv_buf_size = sizeof(struct qrtr_ctrl_pkt);
-	else
-		recv_buf_size += sizeof(struct qmi_header);
 
 	qmi->recv_buf_size = recv_buf_size;
 	qmi->recv_buf = kzalloc(recv_buf_size, GFP_KERNEL);
Signature · 1 line
2.14.1