LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] soc: qcom: qmi: fix a buffer sizing bug
@ 2018-04-27 14:08 Alex Elder
  2018-05-04 23:35 ` Bjorn Andersson
  0 siblings, 1 reply; 2+ messages in thread
From: Alex Elder @ 2018-04-27 14:08 UTC (permalink / raw)
  To: andy.gross, david.brown; +Cc: linux-arm-msm, linux-soc, linux-kernel

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.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/soc/qcom/qmi_interface.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/qcom/qmi_interface.c b/drivers/soc/qcom/qmi_interface.c
index 321982277697..938ca41c56cd 100644
--- a/drivers/soc/qcom/qmi_interface.c
+++ b/drivers/soc/qcom/qmi_interface.c
@@ -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);
-- 
2.14.1

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] soc: qcom: qmi: fix a buffer sizing bug
  2018-04-27 14:08 [PATCH] soc: qcom: qmi: fix a buffer sizing bug Alex Elder
@ 2018-05-04 23:35 ` Bjorn Andersson
  0 siblings, 0 replies; 2+ messages in thread
From: Bjorn Andersson @ 2018-05-04 23:35 UTC (permalink / raw)
  To: Alex Elder
  Cc: andy.gross, david.brown, linux-arm-msm, linux-soc, linux-kernel

On Fri 27 Apr 07:08 PDT 2018, Alex Elder wrote:

> 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.
> 
> Signed-off-by: Alex Elder <elder@linaro.org>

Good catch, thanks!

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  drivers/soc/qcom/qmi_interface.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/qcom/qmi_interface.c b/drivers/soc/qcom/qmi_interface.c
> index 321982277697..938ca41c56cd 100644
> --- a/drivers/soc/qcom/qmi_interface.c
> +++ b/drivers/soc/qcom/qmi_interface.c
> @@ -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);
> -- 
> 2.14.1
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-05-04 23:35 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-27 14:08 [PATCH] soc: qcom: qmi: fix a buffer sizing bug Alex Elder
2018-05-04 23:35 ` Bjorn Andersson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).