| 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: <20240515055631.5775-2-ben@jubnut.com>
Date: Wed, 15 May 2024 06:56:26 +0100
From: Ben Walsh <ben@...nut.com>
To: Benson Leung <bleung@...omium.org>,
Tzung-Bi Shih <tzungbi@...nel.org>,
Guenter Roeck <groeck@...omium.org>,
"Dustin L. Howett" <dustin@...ett.net>,
Kieran Levin <ktl@...me.work>,
Thomas Weißschuh <linux@...ssschuh.net>,
Mario Limonciello <mario.limonciello@....com>,
chrome-platform@...ts.linux.dev,
linux-kernel@...r.kernel.org
Cc: Ben Walsh <ben@...nut.com>
Subject: [PATCH 1/6] platform/chrome: cros_ec_lpc: MEC access can return error code
cros_ec_lpc_io_bytes_mec was returning a u8 checksum of all bytes
read/written, which didn't leave room to indicate errors. Change this
u8 to an int where negative values indicate an error, and non-negative
values are the checksum as before.
Signed-off-by: Ben Walsh <ben@...nut.com>
---
drivers/platform/chrome/cros_ec_lpc.c | 148 ++++++++++++++-------
drivers/platform/chrome/cros_ec_lpc_mec.c | 9 +-
drivers/platform/chrome/cros_ec_lpc_mec.h | 7 +-
drivers/platform/chrome/wilco_ec/mailbox.c | 22 +--
4 files changed, 124 insertions(+), 62 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index ddfbfec44f4c..e638c7d82e22 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -62,14 +62,16 @@ struct cros_ec_lpc {
/**
* struct lpc_driver_ops - LPC driver operations
- * @read: Copy length bytes from EC address offset into buffer dest. Returns
- * the 8-bit checksum of all bytes read.
- * @write: Copy length bytes from buffer msg into EC address offset. Returns
- * the 8-bit checksum of all bytes written.
+ * @read: Copy length bytes from EC address offset into buffer dest.
+ * Returns a negative error code on error, or the 8-bit checksum
+ * of all bytes read.
+ * @write: Copy length bytes from buffer msg into EC address offset.
+ * Returns a negative error code on error, or the 8-bit checksum
+ * of all bytes written.
*/
struct lpc_driver_ops {
- u8 (*read)(unsigned int offset, unsigned int length, u8 *dest);
- u8 (*write)(unsigned int offset, unsigned int length, const u8 *msg);
+ int (*read)(unsigned int offset, unsigned int length, u8 *dest);
+ int (*write)(unsigned int offset, unsigned int length, const u8 *msg);
};
static struct lpc_driver_ops cros_ec_lpc_ops = { };
@@ -78,10 +80,10 @@ static struct lpc_driver_ops cros_ec_lpc_ops = { };
* A generic instance of the read function of struct lpc_driver_ops, used for
* the LPC EC.
*/
-static u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length,
- u8 *dest)
+static int cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length,
+ u8 *dest)
{
- int sum = 0;
+ u8 sum = 0;
int i;
for (i = 0; i < length; ++i) {
@@ -97,10 +99,10 @@ static u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length,
* A generic instance of the write function of struct lpc_driver_ops, used for
* the LPC EC.
*/
-static u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length,
- const u8 *msg)
+static int cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length,
+ const u8 *msg)
{
- int sum = 0;
+ u8 sum = 0;
int i;
for (i = 0; i < length; ++i) {
@@ -116,14 +118,19 @@ static u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length,
* An instance of the read function of struct lpc_driver_ops, used for the
* MEC variant of LPC EC.
*/
-static u8 cros_ec_lpc_mec_read_bytes(unsigned int offset, unsigned int length,
- u8 *dest)
+static int cros_ec_lpc_mec_read_bytes(unsigned int offset, unsigned int length,
+ u8 *dest)
{
- int in_range = cros_ec_lpc_mec_in_range(offset, length);
+ int in_range;
- if (in_range < 0)
+ if (length == 0)
return 0;
+ in_range = cros_ec_lpc_mec_in_range(offset, length);
+
+ if (in_range < 0)
+ return in_range;
+
return in_range ?
cros_ec_lpc_io_bytes_mec(MEC_IO_READ,
offset - EC_HOST_CMD_REGION0,
@@ -135,14 +142,19 @@ static u8 cros_ec_lpc_mec_read_bytes(unsigned int offset, unsigned int length,
* An instance of the write function of struct lpc_driver_ops, used for the
* MEC variant of LPC EC.
*/
-static u8 cros_ec_lpc_mec_write_bytes(unsigned int offset, unsigned int length,
- const u8 *msg)
+static int cros_ec_lpc_mec_write_bytes(unsigned int offset, unsigned int length,
+ const u8 *msg)
{
- int in_range = cros_ec_lpc_mec_in_range(offset, length);
+ int in_range;
- if (in_range < 0)
+ if (length == 0)
return 0;
+ in_range = cros_ec_lpc_mec_in_range(offset, length);
+
+ if (in_range < 0)
+ return in_range;
+
return in_range ?
cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE,
offset - EC_HOST_CMD_REGION0,
@@ -154,11 +166,14 @@ static int ec_response_timed_out(void)
{
unsigned long one_second = jiffies + HZ;
u8 data;
+ int ret;
usleep_range(200, 300);
do {
- if (!(cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_CMD, 1, &data) &
- EC_LPC_STATUS_BUSY_MASK))
+ ret = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_CMD, 1, &data);
+ if (ret < 0)
+ return ret;
+ if (!(data & EC_LPC_STATUS_BUSY_MASK))
return 0;
usleep_range(100, 200);
} while (time_before(jiffies, one_second));
@@ -179,28 +194,41 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
goto done;
/* Write buffer */
- cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_PACKET, ret, ec->dout);
+ ret = cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_PACKET, ret, ec->dout);
+ if (ret < 0)
+ goto done;
/* Here we go */
sum = EC_COMMAND_PROTOCOL_3;
- cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_CMD, 1, &sum);
+ ret = cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_CMD, 1, &sum);
+ if (ret < 0)
+ goto done;
- if (ec_response_timed_out()) {
+ ret = ec_response_timed_out();
+ if (ret < 0)
+ goto done;
+ if (ret) {
dev_warn(ec->dev, "EC response timed out\n");
ret = -EIO;
goto done;
}
/* Check result */
- msg->result = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_DATA, 1, &sum);
+ ret = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_DATA, 1, &sum);
+ if (ret < 0)
+ goto done;
+ msg->result = sum;
ret = cros_ec_check_result(ec, msg);
if (ret)
goto done;
/* Read back response */
dout = (u8 *)&response;
- sum = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_PACKET, sizeof(response),
+ ret = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_PACKET, sizeof(response),
dout);
+ if (ret < 0)
+ goto done;
+ sum = ret;
msg->result = response.result;
@@ -213,9 +241,12 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
}
/* Read response and process checksum */
- sum += cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_PACKET +
- sizeof(response), response.data_len,
- msg->data);
+ ret = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_PACKET +
+ sizeof(response), response.data_len,
+ msg->data);
+ if (ret < 0)
+ goto done;
+ sum += ret;
if (sum) {
dev_err(ec->dev,
@@ -255,32 +286,47 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
sum = msg->command + args.flags + args.command_version + args.data_size;
/* Copy data and update checksum */
- sum += cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_PARAM, msg->outsize,
- msg->data);
+ ret = cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_PARAM, msg->outsize,
+ msg->data);
+ if (ret < 0)
+ goto done;
+ sum += ret;
/* Finalize checksum and write args */
args.checksum = sum;
- cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_ARGS, sizeof(args),
- (u8 *)&args);
+ ret = cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_ARGS, sizeof(args),
+ (u8 *)&args);
+ if (ret < 0)
+ goto done;
/* Here we go */
sum = msg->command;
- cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_CMD, 1, &sum);
+ ret = cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_CMD, 1, &sum);
+ if (ret < 0)
+ goto done;
- if (ec_response_timed_out()) {
+ ret = ec_response_timed_out();
+ if (ret < 0)
+ goto done;
+ if (ret) {
dev_warn(ec->dev, "EC response timed out\n");
ret = -EIO;
goto done;
}
/* Check result */
- msg->result = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_DATA, 1, &sum);
+ ret = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_DATA, 1, &sum);
+ if (ret < 0)
+ goto done;
+ msg->result = sum;
ret = cros_ec_check_result(ec, msg);
if (ret)
goto done;
/* Read back args */
- cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_ARGS, sizeof(args), (u8 *)&args);
+ ret = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_ARGS, sizeof(args), (u8 *)&args);
+ if (ret < 0)
+ goto done;
if (args.data_size > msg->insize) {
dev_err(ec->dev,
@@ -294,8 +340,11 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
sum = msg->command + args.flags + args.command_version + args.data_size;
/* Read response and update checksum */
- sum += cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_PARAM, args.data_size,
- msg->data);
+ ret = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_PARAM, args.data_size,
+ msg->data);
+ if (ret < 0)
+ goto done;
+ sum += ret;
/* Verify checksum */
if (args.checksum != sum) {
@@ -320,19 +369,24 @@ static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset,
int i = offset;
char *s = dest;
int cnt = 0;
+ int ret;
if (offset >= EC_MEMMAP_SIZE - bytes)
return -EINVAL;
/* fixed length */
if (bytes) {
- cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + offset, bytes, s);
+ ret = cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + offset, bytes, s);
+ if (ret < 0)
+ return ret;
return bytes;
}
/* string */
for (; i < EC_MEMMAP_SIZE; i++, s++) {
- cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + i, 1, s);
+ ret = cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + i, 1, s);
+ if (ret < 0)
+ return ret;
cnt++;
if (!*s)
break;
@@ -425,8 +479,8 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
*/
cros_ec_lpc_ops.read = cros_ec_lpc_mec_read_bytes;
cros_ec_lpc_ops.write = cros_ec_lpc_mec_write_bytes;
- cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2, buf);
- if (buf[0] != 'E' || buf[1] != 'C') {
+ ret = cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2, buf);
+ if (ret < 0 || buf[0] != 'E' || buf[1] != 'C') {
if (!devm_request_region(dev, ec_lpc->mmio_memory_base, EC_MEMMAP_SIZE,
dev_name(dev))) {
dev_err(dev, "couldn't reserve memmap region\n");
@@ -436,9 +490,9 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
/* Re-assign read/write operations for the non MEC variant */
cros_ec_lpc_ops.read = cros_ec_lpc_read_bytes;
cros_ec_lpc_ops.write = cros_ec_lpc_write_bytes;
- cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + EC_MEMMAP_ID, 2,
- buf);
- if (buf[0] != 'E' || buf[1] != 'C') {
+ ret = cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + EC_MEMMAP_ID, 2,
+ buf);
+ if (ret < 0 || buf[0] != 'E' || buf[1] != 'C') {
dev_err(dev, "EC ID not detected\n");
return -ENODEV;
}
diff --git a/drivers/platform/chrome/cros_ec_lpc_mec.c b/drivers/platform/chrome/cros_ec_lpc_mec.c
index 0d9c79b270ce..395dc3a6fb5e 100644
--- a/drivers/platform/chrome/cros_ec_lpc_mec.c
+++ b/drivers/platform/chrome/cros_ec_lpc_mec.c
@@ -67,11 +67,12 @@ int cros_ec_lpc_mec_in_range(unsigned int offset, unsigned int length)
* @length: Number of bytes to read / write
* @buf: Destination / source buffer
*
- * Return: 8-bit checksum of all bytes read / written
+ * @return: A negative error code on error, or 8-bit checksum of all
+ * bytes read / written
*/
-u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
- unsigned int offset, unsigned int length,
- u8 *buf)
+int cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
+ unsigned int offset, unsigned int length,
+ u8 *buf)
{
int i = 0;
int io_addr;
diff --git a/drivers/platform/chrome/cros_ec_lpc_mec.h b/drivers/platform/chrome/cros_ec_lpc_mec.h
index 9d0521b23e8a..69670832f187 100644
--- a/drivers/platform/chrome/cros_ec_lpc_mec.h
+++ b/drivers/platform/chrome/cros_ec_lpc_mec.h
@@ -64,9 +64,10 @@ int cros_ec_lpc_mec_in_range(unsigned int offset, unsigned int length);
* @length: Number of bytes to read / write
* @buf: Destination / source buffer
*
- * @return 8-bit checksum of all bytes read / written
+ * @return: A negative error code on error, or 8-bit checksum of all
+ * bytes read / written
*/
-u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
- unsigned int offset, unsigned int length, u8 *buf);
+int cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
+ unsigned int offset, unsigned int length, u8 *buf);
#endif /* __CROS_EC_LPC_MEC_H */
diff --git a/drivers/platform/chrome/wilco_ec/mailbox.c b/drivers/platform/chrome/wilco_ec/mailbox.c
index 0f98358ea824..4d8273b47cde 100644
--- a/drivers/platform/chrome/wilco_ec/mailbox.c
+++ b/drivers/platform/chrome/wilco_ec/mailbox.c
@@ -117,13 +117,17 @@ static int wilco_ec_transfer(struct wilco_ec_device *ec,
struct wilco_ec_request *rq)
{
struct wilco_ec_response *rs;
- u8 checksum;
+ int ret;
u8 flag;
/* Write request header, then data */
- cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, 0, sizeof(*rq), (u8 *)rq);
- cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, sizeof(*rq), msg->request_size,
- msg->request_data);
+ ret = cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, 0, sizeof(*rq), (u8 *)rq);
+ if (ret < 0)
+ return ret;
+ ret = cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, sizeof(*rq), msg->request_size,
+ msg->request_data);
+ if (ret < 0)
+ return ret;
/* Start the command */
outb(EC_MAILBOX_START_COMMAND, ec->io_command->start);
@@ -149,10 +153,12 @@ static int wilco_ec_transfer(struct wilco_ec_device *ec,
/* Read back response */
rs = ec->data_buffer;
- checksum = cros_ec_lpc_io_bytes_mec(MEC_IO_READ, 0,
- sizeof(*rs) + EC_MAILBOX_DATA_SIZE,
- (u8 *)rs);
- if (checksum) {
+ ret = cros_ec_lpc_io_bytes_mec(MEC_IO_READ, 0,
+ sizeof(*rs) + EC_MAILBOX_DATA_SIZE,
+ (u8 *)rs);
+ if (ret < 0)
+ return ret;
+ if (ret) {
dev_dbg(ec->dev, "bad packet checksum 0x%02x\n", rs->checksum);
return -EBADMSG;
}
--
2.45.0
Powered by blists - more mailing lists