| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20220629205939.GA3936194@roeck-us.net>
Date: Wed, 29 Jun 2022 13:59:39 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Eddie James <eajames@...ux.ibm.com>
Cc: linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org,
jdelvare@...e.com
Subject: Re: [PATCH] hwmon: (occ) Prevent power cap command overwriting poll
response
On Tue, Jun 28, 2022 at 03:30:29PM -0500, Eddie James 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@...ux.ibm.com>
Applied.
Thanks,
Guenter
> ---
> 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 | 7 +++----
> 4 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
> index ea070b91e5b9..157b73a3da29 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 64d5ec7e169b..7ac4b2febce6 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 da39ea28df31..b221be1f35f3 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 01405ae2f9bd..c1e0a1d96cd4 100644
> --- a/drivers/hwmon/occ/p9_sbe.c
> +++ b/drivers/hwmon/occ/p9_sbe.c
> @@ -77,11 +77,10 @@ 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 rc;
>
> rc = fsi_occ_submit(ctx->sbe, cmd, len, resp, &resp_len);
> @@ -95,7 +94,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;
Powered by blists - more mailing lists