From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bjorn Andersson Subject: Re: [PATCH] soc: qcom: qmi: fix a buffer sizing bug Date: Fri, 4 May 2018 16:35:16 -0700 Message-ID: <20180504233516.GV18510@minitux> References: <20180427140817.10871-1-elder@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20180427140817.10871-1-elder@linaro.org> Sender: linux-kernel-owner@vger.kernel.org To: Alex Elder Cc: andy.gross@linaro.org, david.brown@linaro.org, linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, linux-kernel@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org 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 Good catch, thanks! Reviewed-by: Bjorn Andersson 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 >