From 8c92a9dbb4915e4f2e0b9538b0006332be5953dc Mon Sep 17 00:00:00 2001 From: Subbaraman Narayanamurthy Date: Tue, 5 May 2020 18:52:31 -0700 Subject: [PATCH] ucsi: ucsi_glink: Fix message handling in ucsi_qti_notify() Currently, ucsi_qti_notify() is checking the received data from charger firmware (PPM) to see if there is any connector status response with partner accessory information (e.g. analog audio). It does this based on setting a flag (cmd_requested_flags) when UCSI_GET_CONNECTOR_STATUS command is sent from UCSI framework to PPM so that the response for that command can be read and clients registered for getting notification on partner accessory can get notified. However, in a certain scenario, multiple response for different commands can be received from PPM. See a possible example below. 1. UCSI framework sends UCSI_GET_CONNECTOR_STATUS command 2. ucsi_glink driver sets cmd_requested_flags, waits for response 3. UCSI framework sends UCSI_GET_CURRENT_CAM command 4. ucsi_glink driver gets a response for UCSI_GET_CONNECTOR_STATUS and schedules notify_work since ucsi_qti_notify() is called with cmd_requested_flags set. 5. ucsi_glink driver gets a response for UCSI_GET_CURRENT_CAM and this can end up reading "flags" because cmd_requested_flags is not cleared yet. However, this causes a buffer overflow because ucsi_qti_read() passes "val" pointer passed from UCSI framework which is of 1 byte length and ucsi_qti_notify() ends up reading "flags" which is at an offset 2 of length 9 bytes. 6. ucsi_qti_notify_work() clears cmd_requested_flags. Fix this by checking the message length in ucsi_qti_notify() to ensure that status->flags is read only when a response is received for UCSI_GET_CONNECTOR_STATUS. Also, relocate the clearing of "cmd_requested_flags" flag. CRs-Fixed: 2678391 Change-Id: Iac1d5c58e1ed2fd0f3bc153da23cddf0700cc097 Signed-off-by: Subbaraman Narayanamurthy --- drivers/usb/typec/ucsi/ucsi_glink.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c index d9dd0a4ead44..785e419e2170 100644 --- a/drivers/usb/typec/ucsi/ucsi_glink.c +++ b/drivers/usb/typec/ucsi/ucsi_glink.c @@ -355,17 +355,17 @@ static void ucsi_qti_notify_work(struct work_struct *work) notify_work); raw_notifier_call_chain(&ucsi_glink_notifier, 0, &udev->constat_info); - mutex_lock(&udev->notify_lock); - clear_bit(CONN_STAT_REQD, &udev->cmd_requested_flags); - mutex_unlock(&udev->notify_lock); } static void ucsi_qti_notify(struct ucsi_dev *udev, unsigned int offset, - struct ucsi_connector_status *status) + struct ucsi_connector_status *status, size_t len) { u8 conn_partner_type, conn_partner_flag; bool cmd_requested; + if (len != sizeof(*status)) + return; + mutex_lock(&udev->notify_lock); cmd_requested = test_bit(CONN_STAT_REQD, &udev->cmd_requested_flags); mutex_unlock(&udev->notify_lock); @@ -403,6 +403,10 @@ static void ucsi_qti_notify(struct ucsi_dev *udev, unsigned int offset, if (conn_partner_flag & UCSI_CONSTAT_PARTNER_FLAG_ALT_MODE) udev->constat_info.partner_alternate_mode = true; + mutex_lock(&udev->notify_lock); + clear_bit(CONN_STAT_REQD, &udev->cmd_requested_flags); + mutex_unlock(&udev->notify_lock); + schedule_work(&udev->notify_work); } } @@ -453,7 +457,7 @@ static int ucsi_qti_read(struct ucsi *ucsi, unsigned int offset, memcpy((u8 *)val, &udev->rx_buf.buf[offset], val_len); atomic_set(&udev->rx_valid, 0); ucsi_log("read:", offset, (u8 *)val, val_len); - ucsi_qti_notify(udev, offset, val); + ucsi_qti_notify(udev, offset, val, val_len); out: mutex_unlock(&udev->read_lock);