[PATCH linux dev-5.15 1/2] hwmon: (occ) Prevent power cap command overwriting poll response
Joel Stanley
joel at jms.id.au
Thu Aug 4 12:55:45 AEST 2022
On Wed, 20 Jul 2022 at 20:16, Eddie James <eajames at linux.ibm.com> wrote:
>
> Currently, the response to the power cap command overwrites the
> first eight bytes of the poll response, since the commands use
> the same buffer. This means that user's get the wrong data between
> the time of sending the power cap and the next poll response update.
> Fix this by specifying a different buffer for the power cap command
> response.
>
> Fixes: 5b5513b88002 ("hwmon: Add On-Chip Controller (OCC) hwmon driver")
> Signed-off-by: Eddie James <eajames at linux.ibm.com>
> Link: https://lore.kernel.org/r/20220628203029.51747-1-eajames@linux.ibm.com
> Signed-off-by: Guenter Roeck <linux at roeck-us.net>
I've applied this and the second patch.
> ---
> drivers/hwmon/occ/common.c | 5 +++--
> drivers/hwmon/occ/common.h | 3 ++-
> drivers/hwmon/occ/p8_i2c.c | 13 +++++++------
> drivers/hwmon/occ/p9_sbe.c | 10 ++++------
> 4 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
> index f00cd59f1d19..1757f3ab842e 100644
> --- a/drivers/hwmon/occ/common.c
> +++ b/drivers/hwmon/occ/common.c
> @@ -145,7 +145,7 @@ static int occ_poll(struct occ *occ)
> cmd[6] = 0; /* checksum lsb */
>
> /* mutex should already be locked if necessary */
> - rc = occ->send_cmd(occ, cmd, sizeof(cmd));
> + rc = occ->send_cmd(occ, cmd, sizeof(cmd), &occ->resp, sizeof(occ->resp));
> if (rc) {
> occ->last_error = rc;
> if (occ->error_count++ > OCC_ERROR_COUNT_THRESHOLD)
> @@ -182,6 +182,7 @@ static int occ_set_user_power_cap(struct occ *occ, u16 user_power_cap)
> {
> int rc;
> u8 cmd[8];
> + u8 resp[8];
> __be16 user_power_cap_be = cpu_to_be16(user_power_cap);
>
> cmd[0] = 0; /* sequence number */
> @@ -198,7 +199,7 @@ static int occ_set_user_power_cap(struct occ *occ, u16 user_power_cap)
> if (rc)
> return rc;
>
> - rc = occ->send_cmd(occ, cmd, sizeof(cmd));
> + rc = occ->send_cmd(occ, cmd, sizeof(cmd), resp, sizeof(resp));
>
> mutex_unlock(&occ->lock);
>
> diff --git a/drivers/hwmon/occ/common.h b/drivers/hwmon/occ/common.h
> index 2dd4a4d240c0..726943af9a07 100644
> --- a/drivers/hwmon/occ/common.h
> +++ b/drivers/hwmon/occ/common.h
> @@ -96,7 +96,8 @@ struct occ {
>
> int powr_sample_time_us; /* average power sample time */
> u8 poll_cmd_data; /* to perform OCC poll command */
> - int (*send_cmd)(struct occ *occ, u8 *cmd, size_t len);
> + int (*send_cmd)(struct occ *occ, u8 *cmd, size_t len, void *resp,
> + size_t resp_len);
>
> unsigned long next_update;
> struct mutex lock; /* lock OCC access */
> diff --git a/drivers/hwmon/occ/p8_i2c.c b/drivers/hwmon/occ/p8_i2c.c
> index 9e61e1fb5142..c35c07964d85 100644
> --- a/drivers/hwmon/occ/p8_i2c.c
> +++ b/drivers/hwmon/occ/p8_i2c.c
> @@ -111,7 +111,8 @@ static int p8_i2c_occ_putscom_be(struct i2c_client *client, u32 address,
> be32_to_cpu(data1));
> }
>
> -static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
> +static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len,
> + void *resp, size_t resp_len)
> {
> int i, rc;
> unsigned long start;
> @@ -120,7 +121,7 @@ static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
> const long wait_time = msecs_to_jiffies(OCC_CMD_IN_PRG_WAIT_MS);
> struct p8_i2c_occ *ctx = to_p8_i2c_occ(occ);
> struct i2c_client *client = ctx->client;
> - struct occ_response *resp = &occ->resp;
> + struct occ_response *or = (struct occ_response *)resp;
>
> start = jiffies;
>
> @@ -151,7 +152,7 @@ static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
> return rc;
>
> /* wait for OCC */
> - if (resp->return_status == OCC_RESP_CMD_IN_PRG) {
> + if (or->return_status == OCC_RESP_CMD_IN_PRG) {
> rc = -EALREADY;
>
> if (time_after(jiffies, start + timeout))
> @@ -163,7 +164,7 @@ static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
> } while (rc);
>
> /* check the OCC response */
> - switch (resp->return_status) {
> + switch (or->return_status) {
> case OCC_RESP_CMD_IN_PRG:
> rc = -ETIMEDOUT;
> break;
> @@ -192,8 +193,8 @@ static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
> if (rc < 0)
> return rc;
>
> - data_length = get_unaligned_be16(&resp->data_length);
> - if (data_length > OCC_RESP_DATA_BYTES)
> + data_length = get_unaligned_be16(&or->data_length);
> + if ((data_length + 7) > resp_len)
> return -EMSGSIZE;
>
> /* fetch the rest of the response data */
> diff --git a/drivers/hwmon/occ/p9_sbe.c b/drivers/hwmon/occ/p9_sbe.c
> index d34d6a007e8d..ad2fcc31db78 100644
> --- a/drivers/hwmon/occ/p9_sbe.c
> +++ b/drivers/hwmon/occ/p9_sbe.c
> @@ -79,13 +79,11 @@ static bool p9_sbe_occ_save_ffdc(struct p9_sbe_occ *ctx, const void *resp,
> return notify;
> }
>
> -static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
> +static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len,
> + void *resp, size_t resp_len)
> {
> - struct occ_response *resp = &occ->resp;
> struct p9_sbe_occ *ctx = to_p9_sbe_occ(occ);
> - size_t resp_len = sizeof(*resp);
> - int i;
> - int rc;
> + int rc, i;
>
> for (i = 0; i < OCC_CHECKSUM_RETRIES; ++i) {
> rc = fsi_occ_submit(ctx->sbe, cmd, len, resp, &resp_len);
> @@ -102,7 +100,7 @@ static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
> return rc;
> }
>
> - switch (resp->return_status) {
> + switch (((struct occ_response *)resp)->return_status) {
> case OCC_RESP_CMD_IN_PRG:
> rc = -ETIMEDOUT;
> break;
> --
> 2.31.1
>
More information about the openbmc
mailing list