diff options
| author | Wai-Hong Tam <waihong@google.com> | 2020-06-24 10:02:45 -0700 |
|---|---|---|
| committer | Commit Bot <commit-bot@chromium.org> | 2020-06-25 04:48:11 +0000 |
| commit | 1620833b5e7c97a1f0d2a7007746e1f961d7587f (patch) | |
| tree | c269d7766191d636f1dc646555a4724b97be6b22 /common | |
| parent | d945012134ae499ff8903739d5472f5cdd51703a (diff) | |
| download | chrome-ec-1620833b5e7c97a1f0d2a7007746e1f961d7587f.tar.gz | |
tcpm: Change the get_chip_info() to prevent race conditions
The original get_chip_info() returns a point of point to the chip_info.
This way helps to cache the chip_info to a static variable and the
function just returns the pointer to the static variable.
This static variable has a race condition on the PS8805 chip. The PS8805
chip returns a different PID when the firmware is corrupted, i.e. 0x8803
instead of 0x8805. The !live case fixes the PID, by modifying the static
variable directly. When another task calls the same function for the
live case, the static variable is modified and has a race condition.
This change fixes the issue by changing the get_chip_info() parameter to
a point of the chip_info. The caller has to allocate a buffer in the
stack and pass the address to the function. For the !live case, the
function copies the cache value from the static variable to the buffer.
So the static variable doesn't have a race condition.
BRANCH=None
BUG=b:159588335
TEST=Used ectool to check the PD chip PID 0x8805 (was 0x8803).
localhost ~ # ectool pdchipinfo 1
vendor_id: 0x1da0
product_id: 0x8805
device_id: 0x1
fw_version: 0x0
min_req_fw_version: 0x0
Change-Id: Ic24615af77ea58016d286480572d2a282c4fa09a
Signed-off-by: Wai-Hong Tam <waihong@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2264477
Reviewed-by: Julius Werner <jwerner@chromium.org>
Diffstat (limited to 'common')
| -rw-r--r-- | common/mock/tcpc_mock.c | 2 | ||||
| -rw-r--r-- | common/usb_pd_host_cmd.c | 4 | ||||
| -rw-r--r-- | common/usb_pd_protocol.c | 6 |
3 files changed, 6 insertions, 6 deletions
diff --git a/common/mock/tcpc_mock.c b/common/mock/tcpc_mock.c index 8d62710f6a..27b79af989 100644 --- a/common/mock/tcpc_mock.c +++ b/common/mock/tcpc_mock.c @@ -146,7 +146,7 @@ __maybe_unused static int mock_drp_toggle(int port) } static int mock_get_chip_info(int port, int live, - struct ec_response_pd_chip_info_v1 **info) + struct ec_response_pd_chip_info_v1 *info) { return EC_SUCCESS; } diff --git a/common/usb_pd_host_cmd.c b/common/usb_pd_host_cmd.c index 6e8afe761e..3a26018e64 100644 --- a/common/usb_pd_host_cmd.c +++ b/common/usb_pd_host_cmd.c @@ -89,7 +89,7 @@ DECLARE_HOST_COMMAND(EC_CMD_USB_PD_RW_HASH_ENTRY, static enum ec_status hc_remote_pd_chip_info(struct host_cmd_handler_args *args) { const struct ec_params_pd_chip_info *p = args->params; - struct ec_response_pd_chip_info_v1 *info; + struct ec_response_pd_chip_info_v1 info; if (p->port >= board_get_usb_pd_port_count()) return EC_RES_INVALID_PARAM; @@ -105,7 +105,7 @@ static enum ec_status hc_remote_pd_chip_info(struct host_cmd_handler_args *args) args->version ? sizeof(struct ec_response_pd_chip_info_v1) : sizeof(struct ec_response_pd_chip_info); - memcpy(args->response, info, args->response_size); + memcpy(args->response, &info, args->response_size); return EC_RES_SUCCESS; } diff --git a/common/usb_pd_protocol.c b/common/usb_pd_protocol.c index 151e4b50aa..31093b9a2d 100644 --- a/common/usb_pd_protocol.c +++ b/common/usb_pd_protocol.c @@ -2940,14 +2940,14 @@ void pd_task(void *u) this_state = res ? PD_STATE_SUSPENDED : PD_DEFAULT_STATE(port); #ifndef CONFIG_USB_PD_TCPC if (!res) { - struct ec_response_pd_chip_info_v1 *info; + struct ec_response_pd_chip_info_v1 info; if (tcpm_get_chip_info(port, 0, &info) == EC_SUCCESS) { CPRINTS("TCPC p%d VID:0x%x PID:0x%x DID:0x%x " "FWV:0x%" PRIx64, - port, info->vendor_id, info->product_id, - info->device_id, info->fw_version_number); + port, info.vendor_id, info.product_id, + info.device_id, info.fw_version_number); } } #endif |