| Message ID | 20240416-ucsi-glink-altmode-v1-3-890db00877ac@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | usb: typec: ucsi: glink: merge in altmode support | expand |
On 4/16/24 04:20, Dmitry Baryshkov wrote: > The driver gets data from the DSP firmware. Sanitize data size before > reading corresponding message structures. > > Fixes: 62b5412b1f4a ("usb: typec: ucsi: add PMIC Glink UCSI driver") > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- I think more backstory would be beneficial here.. Does this happen often? What are the consequences? What are the causes? Can there be one-off invalid messages, or does that mean the firwmare has entered some unstable state? And I suppose, if answer to the last question is "unstable state", are we doing something incorrectly in Linux that causes it to happen? Konrad
On Tue, 16 Apr 2024 at 17:33, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > > On 4/16/24 04:20, Dmitry Baryshkov wrote: > > The driver gets data from the DSP firmware. Sanitize data size before > > reading corresponding message structures. > > > > Fixes: 62b5412b1f4a ("usb: typec: ucsi: add PMIC Glink UCSI driver") > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > I think more backstory would be beneficial here.. Does this happen often? > What are the consequences? What are the causes? Can there be one-off invalid > messages, or does that mean the firwmare has entered some unstable state? > > And I suppose, if answer to the last question is "unstable state", are we > doing something incorrectly in Linux that causes it to happen? No. I haven't seen such cases. However as we are getting the data from external entity which don't control, we should check that the messages conform to what we are expecting.
diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c index f7546bb488c3..6be9d89d4a28 100644 --- a/drivers/usb/typec/ucsi/ucsi_glink.c +++ b/drivers/usb/typec/ucsi/ucsi_glink.c @@ -223,9 +223,16 @@ static const struct ucsi_operations pmic_glink_ucsi_ops = { .connector_status = pmic_glink_ucsi_connector_status, }; -static void pmic_glink_ucsi_read_ack(struct pmic_glink_ucsi *ucsi, const void *data, int len) +static void pmic_glink_ucsi_read_ack(struct pmic_glink_ucsi *ucsi, const void *data, size_t len) { - const struct ucsi_read_buf_resp_msg *resp = data; + const struct ucsi_read_buf_resp_msg *resp; + + if (len != sizeof (*resp)) { + dev_err_ratelimited(ucsi->dev, "Unexpected read response struct size %zd\n", len); + return; + } + + resp = data; if (resp->ret_code) return; @@ -234,9 +241,16 @@ static void pmic_glink_ucsi_read_ack(struct pmic_glink_ucsi *ucsi, const void *d complete(&ucsi->read_ack); } -static void pmic_glink_ucsi_write_ack(struct pmic_glink_ucsi *ucsi, const void *data, int len) +static void pmic_glink_ucsi_write_ack(struct pmic_glink_ucsi *ucsi, const void *data, size_t len) { - const struct ucsi_write_buf_resp_msg *resp = data; + const struct ucsi_write_buf_resp_msg *resp; + + if (len != sizeof (*resp)) { + dev_err_ratelimited(ucsi->dev, "Unexpected write ack struct size %zd\n", len); + return; + } + + resp = data; if (resp->ret_code) return;
The driver gets data from the DSP firmware. Sanitize data size before reading corresponding message structures. Fixes: 62b5412b1f4a ("usb: typec: ucsi: add PMIC Glink UCSI driver") Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/usb/typec/ucsi/ucsi_glink.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)