* [PATCH v2 0/7] platform/chrome: get rid of BUG_ON()
@ 2022-05-13 4:41 Tzung-Bi Shih
2022-05-13 4:41 ` [PATCH v2 1/7] platform/chrome: cros_ec_proto: drop unneeded BUG_ON() in prepare_packet() Tzung-Bi Shih
` (8 more replies)
0 siblings, 9 replies; 12+ messages in thread
From: Tzung-Bi Shih @ 2022-05-13 4:41 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, tzungbi, linux-kernel
The series gets rid of BUG_ON()s in drivers/platform/chrome/. Most of them
can be replaced by returning proper return code.
The 2nd patch makes callers of cros_ec_prepare_tx() to take care of the
return code.
The 3rd patch turns cros_ec_prepare_tx() to return error code if any.
Changes from v1:
(https://patchwork.kernel.org/project/chrome-platform/cover/20220512083627.885338-1-tzungbi@kernel.org/)
- Split the original 6th patch into 2 smaller patches.
Tzung-Bi Shih (7):
platform/chrome: cros_ec_proto: drop unneeded BUG_ON() in
prepare_packet()
platform/chrome: correct cros_ec_prepare_tx() usage
platform/chrome: cros_ec_proto: drop BUG_ON() in cros_ec_prepare_tx()
platform/chrome: cros_ec_proto: drop BUG_ON() in
cros_ec_get_host_event()
platform/chrome: cros_ec_i2c: drop BUG_ON() in cros_ec_pkt_xfer_i2c()
platform/chrome: cros_ec_spi: drop unneeded BUG_ON()
platform/chrome: cros_ec_spi: drop BUG_ON() if `din` isn't large
enough
drivers/platform/chrome/cros_ec_i2c.c | 12 ++++++++++--
drivers/platform/chrome/cros_ec_ishtp.c | 4 +++-
drivers/platform/chrome/cros_ec_lpc.c | 2 ++
drivers/platform/chrome/cros_ec_proto.c | 13 ++++++++-----
drivers/platform/chrome/cros_ec_rpmsg.c | 2 ++
drivers/platform/chrome/cros_ec_spi.c | 15 ++++++++++-----
6 files changed, 35 insertions(+), 13 deletions(-)
--
2.36.0.550.gb090851708-goog
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/7] platform/chrome: cros_ec_proto: drop unneeded BUG_ON() in prepare_packet()
2022-05-13 4:41 [PATCH v2 0/7] platform/chrome: get rid of BUG_ON() Tzung-Bi Shih
@ 2022-05-13 4:41 ` Tzung-Bi Shih
2022-05-13 4:41 ` [PATCH v2 2/7] platform/chrome: correct cros_ec_prepare_tx() usage Tzung-Bi Shih
` (7 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Tzung-Bi Shih @ 2022-05-13 4:41 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, tzungbi, linux-kernel
prepare_packet() gets called if `ec_dev->proto_version` > 2. For now, it
must be equivalent to EC_HOST_REQUEST_VERSION.
Drop the BUG_ON().
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Add R-b tag.
drivers/platform/chrome/cros_ec_proto.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index ac1419881ff3..db1c8ba43171 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -60,7 +60,6 @@ static int prepare_packet(struct cros_ec_device *ec_dev,
int i;
u8 csum = 0;
- BUG_ON(ec_dev->proto_version != EC_HOST_REQUEST_VERSION);
BUG_ON(msg->outsize + sizeof(*request) > ec_dev->dout_size);
out = ec_dev->dout;
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/7] platform/chrome: correct cros_ec_prepare_tx() usage
2022-05-13 4:41 [PATCH v2 0/7] platform/chrome: get rid of BUG_ON() Tzung-Bi Shih
2022-05-13 4:41 ` [PATCH v2 1/7] platform/chrome: cros_ec_proto: drop unneeded BUG_ON() in prepare_packet() Tzung-Bi Shih
@ 2022-05-13 4:41 ` Tzung-Bi Shih
2022-05-13 4:41 ` [PATCH v2 3/7] platform/chrome: cros_ec_proto: drop BUG_ON() in cros_ec_prepare_tx() Tzung-Bi Shih
` (6 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Tzung-Bi Shih @ 2022-05-13 4:41 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, tzungbi, linux-kernel
cros_ec_prepare_tx() returns either:
- >= 0 for number of prepared bytes.
- < 0 for -errno.
Correct the comment and make sure all callers check the return code.
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Add R-b tag.
drivers/platform/chrome/cros_ec_i2c.c | 2 ++
drivers/platform/chrome/cros_ec_ishtp.c | 4 +++-
drivers/platform/chrome/cros_ec_lpc.c | 2 ++
drivers/platform/chrome/cros_ec_proto.c | 2 +-
drivers/platform/chrome/cros_ec_rpmsg.c | 2 ++
drivers/platform/chrome/cros_ec_spi.c | 4 ++++
6 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_i2c.c b/drivers/platform/chrome/cros_ec_i2c.c
index 22feb0fd4ce7..a4f305f1eb0e 100644
--- a/drivers/platform/chrome/cros_ec_i2c.c
+++ b/drivers/platform/chrome/cros_ec_i2c.c
@@ -89,6 +89,8 @@ static int cros_ec_pkt_xfer_i2c(struct cros_ec_device *ec_dev,
ec_dev->dout++;
ret = cros_ec_prepare_tx(ec_dev, msg);
+ if (ret < 0)
+ goto done;
ec_dev->dout--;
/* send command to EC and read answer */
diff --git a/drivers/platform/chrome/cros_ec_ishtp.c b/drivers/platform/chrome/cros_ec_ishtp.c
index 4020b8354bae..cb2031cf7106 100644
--- a/drivers/platform/chrome/cros_ec_ishtp.c
+++ b/drivers/platform/chrome/cros_ec_ishtp.c
@@ -521,7 +521,9 @@ static int cros_ec_pkt_xfer_ish(struct cros_ec_device *ec_dev,
out_msg->hdr.status = 0;
ec_dev->dout += OUT_MSG_EC_REQUEST_PREAMBLE;
- cros_ec_prepare_tx(ec_dev, msg);
+ rv = cros_ec_prepare_tx(ec_dev, msg);
+ if (rv < 0)
+ goto end_error;
ec_dev->dout -= OUT_MSG_EC_REQUEST_PREAMBLE;
dev_dbg(dev,
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 8eeef85a96b1..7677ab3c0ead 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -147,6 +147,8 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
u8 *dout;
ret = cros_ec_prepare_tx(ec, msg);
+ if (ret < 0)
+ goto done;
/* Write buffer */
cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_PACKET, ret, ec->dout);
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index db1c8ba43171..2d6d3fbfa905 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -164,7 +164,7 @@ static int send_command(struct cros_ec_device *ec_dev,
* only SPI uses it. Once LPC uses the same protocol it can start using it.
* I2C could use it now, with a refactor of the existing code.
*
- * Return: 0 on success or negative error code.
+ * Return: number of prepared bytes on success or negative error code.
*/
int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg)
diff --git a/drivers/platform/chrome/cros_ec_rpmsg.c b/drivers/platform/chrome/cros_ec_rpmsg.c
index d96d15b8ca94..39d3b50a7c09 100644
--- a/drivers/platform/chrome/cros_ec_rpmsg.c
+++ b/drivers/platform/chrome/cros_ec_rpmsg.c
@@ -89,6 +89,8 @@ static int cros_ec_pkt_xfer_rpmsg(struct cros_ec_device *ec_dev,
ec_msg->result = 0;
len = cros_ec_prepare_tx(ec_dev, ec_msg);
+ if (len < 0)
+ return len;
dev_dbg(ec_dev->dev, "prepared, len=%d\n", len);
reinit_completion(&ec_rpmsg->xfer_ack);
diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c
index 8493af0f680e..589f18e9537d 100644
--- a/drivers/platform/chrome/cros_ec_spi.c
+++ b/drivers/platform/chrome/cros_ec_spi.c
@@ -401,6 +401,8 @@ static int do_cros_ec_pkt_xfer_spi(struct cros_ec_device *ec_dev,
unsigned long delay;
len = cros_ec_prepare_tx(ec_dev, ec_msg);
+ if (len < 0)
+ return len;
dev_dbg(ec_dev->dev, "prepared, len=%d\n", len);
/* If it's too soon to do another transaction, wait */
@@ -544,6 +546,8 @@ static int do_cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
unsigned long delay;
len = cros_ec_prepare_tx(ec_dev, ec_msg);
+ if (len < 0)
+ return len;
dev_dbg(ec_dev->dev, "prepared, len=%d\n", len);
/* If it's too soon to do another transaction, wait */
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/7] platform/chrome: cros_ec_proto: drop BUG_ON() in cros_ec_prepare_tx()
2022-05-13 4:41 [PATCH v2 0/7] platform/chrome: get rid of BUG_ON() Tzung-Bi Shih
2022-05-13 4:41 ` [PATCH v2 1/7] platform/chrome: cros_ec_proto: drop unneeded BUG_ON() in prepare_packet() Tzung-Bi Shih
2022-05-13 4:41 ` [PATCH v2 2/7] platform/chrome: correct cros_ec_prepare_tx() usage Tzung-Bi Shih
@ 2022-05-13 4:41 ` Tzung-Bi Shih
2022-05-13 4:41 ` [PATCH v2 4/7] platform/chrome: cros_ec_proto: drop BUG_ON() in cros_ec_get_host_event() Tzung-Bi Shih
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Tzung-Bi Shih @ 2022-05-13 4:41 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, tzungbi, linux-kernel
It is overkill to crash the kernel if the given message is oversize.
Drop the BUG_ON() and return -EINVAL instead.
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Add R-b tag.
drivers/platform/chrome/cros_ec_proto.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 2d6d3fbfa905..9ce3374846ff 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -60,7 +60,8 @@ static int prepare_packet(struct cros_ec_device *ec_dev,
int i;
u8 csum = 0;
- BUG_ON(msg->outsize + sizeof(*request) > ec_dev->dout_size);
+ if (msg->outsize + sizeof(*request) > ec_dev->dout_size)
+ return -EINVAL;
out = ec_dev->dout;
request = (struct ec_host_request *)out;
@@ -176,7 +177,9 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
if (ec_dev->proto_version > 2)
return prepare_packet(ec_dev, msg);
- BUG_ON(msg->outsize > EC_PROTO2_MAX_PARAM_SIZE);
+ if (msg->outsize > EC_PROTO2_MAX_PARAM_SIZE)
+ return -EINVAL;
+
out = ec_dev->dout;
out[0] = EC_CMD_VERSION0 + msg->version;
out[1] = msg->command;
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 4/7] platform/chrome: cros_ec_proto: drop BUG_ON() in cros_ec_get_host_event()
2022-05-13 4:41 [PATCH v2 0/7] platform/chrome: get rid of BUG_ON() Tzung-Bi Shih
` (2 preceding siblings ...)
2022-05-13 4:41 ` [PATCH v2 3/7] platform/chrome: cros_ec_proto: drop BUG_ON() in cros_ec_prepare_tx() Tzung-Bi Shih
@ 2022-05-13 4:41 ` Tzung-Bi Shih
2022-05-13 4:41 ` [PATCH v2 5/7] platform/chrome: cros_ec_i2c: drop BUG_ON() in cros_ec_pkt_xfer_i2c() Tzung-Bi Shih
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Tzung-Bi Shih @ 2022-05-13 4:41 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, tzungbi, linux-kernel
It is overkill to crash the kernel if the `ec_dev` doesn't support MKBP
event but gets called into cros_ec_get_host_event().
Drop the BUG_ON() and return error (0 in the case) instead.
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Add R-b tag.
drivers/platform/chrome/cros_ec_proto.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 9ce3374846ff..ff767dccdf0f 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -817,7 +817,8 @@ u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev)
{
u32 host_event;
- BUG_ON(!ec_dev->mkbp_event_supported);
+ if (!ec_dev->mkbp_event_supported)
+ return 0;
if (ec_dev->event_data.event_type != EC_MKBP_EVENT_HOST_EVENT)
return 0;
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 5/7] platform/chrome: cros_ec_i2c: drop BUG_ON() in cros_ec_pkt_xfer_i2c()
2022-05-13 4:41 [PATCH v2 0/7] platform/chrome: get rid of BUG_ON() Tzung-Bi Shih
` (3 preceding siblings ...)
2022-05-13 4:41 ` [PATCH v2 4/7] platform/chrome: cros_ec_proto: drop BUG_ON() in cros_ec_get_host_event() Tzung-Bi Shih
@ 2022-05-13 4:41 ` Tzung-Bi Shih
2022-05-13 4:41 ` [PATCH v2 6/7] platform/chrome: cros_ec_spi: drop unneeded BUG_ON() Tzung-Bi Shih
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Tzung-Bi Shih @ 2022-05-13 4:41 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, tzungbi, linux-kernel
It is overkill to crash the kernel if the given message is oversize.
Drop the BUG_ON() and return -EINVAL instead.
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Add R-b tag.
drivers/platform/chrome/cros_ec_i2c.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_i2c.c b/drivers/platform/chrome/cros_ec_i2c.c
index a4f305f1eb0e..9f5b95763173 100644
--- a/drivers/platform/chrome/cros_ec_i2c.c
+++ b/drivers/platform/chrome/cros_ec_i2c.c
@@ -72,13 +72,19 @@ static int cros_ec_pkt_xfer_i2c(struct cros_ec_device *ec_dev,
i2c_msg[1].flags = I2C_M_RD;
packet_len = msg->insize + response_header_size;
- BUG_ON(packet_len > ec_dev->din_size);
+ if (packet_len > ec_dev->din_size) {
+ ret = -EINVAL;
+ goto done;
+ }
in_buf = ec_dev->din;
i2c_msg[1].len = packet_len;
i2c_msg[1].buf = (char *) in_buf;
packet_len = msg->outsize + request_header_size;
- BUG_ON(packet_len > ec_dev->dout_size);
+ if (packet_len > ec_dev->dout_size) {
+ ret = -EINVAL;
+ goto done;
+ }
out_buf = ec_dev->dout;
i2c_msg[0].len = packet_len;
i2c_msg[0].buf = (char *) out_buf;
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 6/7] platform/chrome: cros_ec_spi: drop unneeded BUG_ON()
2022-05-13 4:41 [PATCH v2 0/7] platform/chrome: get rid of BUG_ON() Tzung-Bi Shih
` (4 preceding siblings ...)
2022-05-13 4:41 ` [PATCH v2 5/7] platform/chrome: cros_ec_i2c: drop BUG_ON() in cros_ec_pkt_xfer_i2c() Tzung-Bi Shih
@ 2022-05-13 4:41 ` Tzung-Bi Shih
2022-05-13 13:19 ` Guenter Roeck
2022-05-13 4:41 ` [PATCH v2 7/7] platform/chrome: cros_ec_spi: drop BUG_ON() if `din` isn't large enough Tzung-Bi Shih
` (2 subsequent siblings)
8 siblings, 1 reply; 12+ messages in thread
From: Tzung-Bi Shih @ 2022-05-13 4:41 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, tzungbi, linux-kernel
In the context, the following conditions are always false:
- `todo` < 0
Suppose that EC_SPI_FRAME_START is found at the last byte of transfer.
In the case, `ptr` == `end` - 1. As a result, `todo` must be 0.
- `todo` > `ec_dev->din_size`
Suppose that there is no preamble bytes. EC_SPI_FRAME_START is found at
the first byte of transfer.
In the case, `end` == `ptr` + EC_MSG_PREAMBLE_COUNT.
As a result, `todo` == EC_MSG_PREAMBLE_COUNT - 1.
However, it already checked `ec_dev->din_size` < EC_MSG_PREAMBLE_COUNT at
the beginning of function.
Drop the unneeded BUG_ON().
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Separate from the original 6th patch.
- Drop the BUG_ON() instead of returning -EINVAL.
drivers/platform/chrome/cros_ec_spi.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c
index 589f18e9537d..5264615f46af 100644
--- a/drivers/platform/chrome/cros_ec_spi.c
+++ b/drivers/platform/chrome/cros_ec_spi.c
@@ -237,7 +237,6 @@ static int cros_ec_spi_receive_packet(struct cros_ec_device *ec_dev,
* start of our buffer
*/
todo = end - ++ptr;
- BUG_ON(todo < 0 || todo > ec_dev->din_size);
todo = min(todo, need_len);
memmove(ec_dev->din, ptr, todo);
ptr = ec_dev->din + todo;
@@ -345,7 +344,6 @@ static int cros_ec_spi_receive_response(struct cros_ec_device *ec_dev,
* start of our buffer
*/
todo = end - ++ptr;
- BUG_ON(todo < 0 || todo > ec_dev->din_size);
todo = min(todo, need_len);
memmove(ec_dev->din, ptr, todo);
ptr = ec_dev->din + todo;
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 7/7] platform/chrome: cros_ec_spi: drop BUG_ON() if `din` isn't large enough
2022-05-13 4:41 [PATCH v2 0/7] platform/chrome: get rid of BUG_ON() Tzung-Bi Shih
` (5 preceding siblings ...)
2022-05-13 4:41 ` [PATCH v2 6/7] platform/chrome: cros_ec_spi: drop unneeded BUG_ON() Tzung-Bi Shih
@ 2022-05-13 4:41 ` Tzung-Bi Shih
2022-05-13 13:20 ` Guenter Roeck
2022-05-16 2:10 ` [PATCH v2 0/7] platform/chrome: get rid of BUG_ON() patchwork-bot+chrome-platform
2022-05-18 5:20 ` patchwork-bot+chrome-platform
8 siblings, 1 reply; 12+ messages in thread
From: Tzung-Bi Shih @ 2022-05-13 4:41 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, tzungbi, linux-kernel
It is overkill to crash the kernel if the `din` buffer is going to full
or overflow.
Drop the BUG_ON() and return -EINVAL instead.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Separate from the original 6th patch.
drivers/platform/chrome/cros_ec_spi.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c
index 5264615f46af..7360b3ff6e4f 100644
--- a/drivers/platform/chrome/cros_ec_spi.c
+++ b/drivers/platform/chrome/cros_ec_spi.c
@@ -160,7 +160,8 @@ static int receive_n_bytes(struct cros_ec_device *ec_dev, u8 *buf, int n)
struct spi_message msg;
int ret;
- BUG_ON(buf - ec_dev->din + n > ec_dev->din_size);
+ if (buf - ec_dev->din + n > ec_dev->din_size)
+ return -EINVAL;
memset(&trans, 0, sizeof(trans));
trans.cs_change = 1;
@@ -197,7 +198,8 @@ static int cros_ec_spi_receive_packet(struct cros_ec_device *ec_dev,
unsigned long deadline;
int todo;
- BUG_ON(ec_dev->din_size < EC_MSG_PREAMBLE_COUNT);
+ if (ec_dev->din_size < EC_MSG_PREAMBLE_COUNT)
+ return -EINVAL;
/* Receive data until we see the header byte */
deadline = jiffies + msecs_to_jiffies(EC_MSG_DEADLINE_MS);
@@ -304,7 +306,8 @@ static int cros_ec_spi_receive_response(struct cros_ec_device *ec_dev,
unsigned long deadline;
int todo;
- BUG_ON(ec_dev->din_size < EC_MSG_PREAMBLE_COUNT);
+ if (ec_dev->din_size < EC_MSG_PREAMBLE_COUNT)
+ return -EINVAL;
/* Receive data until we see the header byte */
deadline = jiffies + msecs_to_jiffies(EC_MSG_DEADLINE_MS);
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 6/7] platform/chrome: cros_ec_spi: drop unneeded BUG_ON()
2022-05-13 4:41 ` [PATCH v2 6/7] platform/chrome: cros_ec_spi: drop unneeded BUG_ON() Tzung-Bi Shih
@ 2022-05-13 13:19 ` Guenter Roeck
0 siblings, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2022-05-13 13:19 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: Benson Leung, Guenter Roeck, chrome-platform, linux-kernel
On Thu, May 12, 2022 at 9:42 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> In the context, the following conditions are always false:
>
> - `todo` < 0
> Suppose that EC_SPI_FRAME_START is found at the last byte of transfer.
>
> In the case, `ptr` == `end` - 1. As a result, `todo` must be 0.
>
> - `todo` > `ec_dev->din_size`
> Suppose that there is no preamble bytes. EC_SPI_FRAME_START is found at
> the first byte of transfer.
>
> In the case, `end` == `ptr` + EC_MSG_PREAMBLE_COUNT.
> As a result, `todo` == EC_MSG_PREAMBLE_COUNT - 1.
> However, it already checked `ec_dev->din_size` < EC_MSG_PREAMBLE_COUNT at
> the beginning of function.
>
> Drop the unneeded BUG_ON().
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
> ---
> Changes from v1:
> - Separate from the original 6th patch.
> - Drop the BUG_ON() instead of returning -EINVAL.
>
> drivers/platform/chrome/cros_ec_spi.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c
> index 589f18e9537d..5264615f46af 100644
> --- a/drivers/platform/chrome/cros_ec_spi.c
> +++ b/drivers/platform/chrome/cros_ec_spi.c
> @@ -237,7 +237,6 @@ static int cros_ec_spi_receive_packet(struct cros_ec_device *ec_dev,
> * start of our buffer
> */
> todo = end - ++ptr;
> - BUG_ON(todo < 0 || todo > ec_dev->din_size);
> todo = min(todo, need_len);
> memmove(ec_dev->din, ptr, todo);
> ptr = ec_dev->din + todo;
> @@ -345,7 +344,6 @@ static int cros_ec_spi_receive_response(struct cros_ec_device *ec_dev,
> * start of our buffer
> */
> todo = end - ++ptr;
> - BUG_ON(todo < 0 || todo > ec_dev->din_size);
> todo = min(todo, need_len);
> memmove(ec_dev->din, ptr, todo);
> ptr = ec_dev->din + todo;
> --
> 2.36.0.550.gb090851708-goog
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 7/7] platform/chrome: cros_ec_spi: drop BUG_ON() if `din` isn't large enough
2022-05-13 4:41 ` [PATCH v2 7/7] platform/chrome: cros_ec_spi: drop BUG_ON() if `din` isn't large enough Tzung-Bi Shih
@ 2022-05-13 13:20 ` Guenter Roeck
0 siblings, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2022-05-13 13:20 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: Benson Leung, Guenter Roeck, chrome-platform, linux-kernel
On Thu, May 12, 2022 at 9:42 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> It is overkill to crash the kernel if the `din` buffer is going to full
> or overflow.
>
> Drop the BUG_ON() and return -EINVAL instead.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
> ---
> Changes from v1:
> - Separate from the original 6th patch.
>
> drivers/platform/chrome/cros_ec_spi.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c
> index 5264615f46af..7360b3ff6e4f 100644
> --- a/drivers/platform/chrome/cros_ec_spi.c
> +++ b/drivers/platform/chrome/cros_ec_spi.c
> @@ -160,7 +160,8 @@ static int receive_n_bytes(struct cros_ec_device *ec_dev, u8 *buf, int n)
> struct spi_message msg;
> int ret;
>
> - BUG_ON(buf - ec_dev->din + n > ec_dev->din_size);
> + if (buf - ec_dev->din + n > ec_dev->din_size)
> + return -EINVAL;
>
> memset(&trans, 0, sizeof(trans));
> trans.cs_change = 1;
> @@ -197,7 +198,8 @@ static int cros_ec_spi_receive_packet(struct cros_ec_device *ec_dev,
> unsigned long deadline;
> int todo;
>
> - BUG_ON(ec_dev->din_size < EC_MSG_PREAMBLE_COUNT);
> + if (ec_dev->din_size < EC_MSG_PREAMBLE_COUNT)
> + return -EINVAL;
>
> /* Receive data until we see the header byte */
> deadline = jiffies + msecs_to_jiffies(EC_MSG_DEADLINE_MS);
> @@ -304,7 +306,8 @@ static int cros_ec_spi_receive_response(struct cros_ec_device *ec_dev,
> unsigned long deadline;
> int todo;
>
> - BUG_ON(ec_dev->din_size < EC_MSG_PREAMBLE_COUNT);
> + if (ec_dev->din_size < EC_MSG_PREAMBLE_COUNT)
> + return -EINVAL;
>
> /* Receive data until we see the header byte */
> deadline = jiffies + msecs_to_jiffies(EC_MSG_DEADLINE_MS);
> --
> 2.36.0.550.gb090851708-goog
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/7] platform/chrome: get rid of BUG_ON()
2022-05-13 4:41 [PATCH v2 0/7] platform/chrome: get rid of BUG_ON() Tzung-Bi Shih
` (6 preceding siblings ...)
2022-05-13 4:41 ` [PATCH v2 7/7] platform/chrome: cros_ec_spi: drop BUG_ON() if `din` isn't large enough Tzung-Bi Shih
@ 2022-05-16 2:10 ` patchwork-bot+chrome-platform
2022-05-18 5:20 ` patchwork-bot+chrome-platform
8 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+chrome-platform @ 2022-05-16 2:10 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform, linux-kernel
Hello:
This series was applied to chrome-platform/linux.git (for-kernelci)
by Tzung-Bi Shih <tzungbi@kernel.org>:
On Fri, 13 May 2022 12:41:36 +0800 you wrote:
> The series gets rid of BUG_ON()s in drivers/platform/chrome/. Most of them
> can be replaced by returning proper return code.
>
> The 2nd patch makes callers of cros_ec_prepare_tx() to take care of the
> return code.
>
> The 3rd patch turns cros_ec_prepare_tx() to return error code if any.
>
> [...]
Here is the summary with links:
- [v2,1/7] platform/chrome: cros_ec_proto: drop unneeded BUG_ON() in prepare_packet()
https://git.kernel.org/chrome-platform/c/42701e7c0cd2
- [v2,2/7] platform/chrome: correct cros_ec_prepare_tx() usage
https://git.kernel.org/chrome-platform/c/71d3ae7fb640
- [v2,3/7] platform/chrome: cros_ec_proto: drop BUG_ON() in cros_ec_prepare_tx()
https://git.kernel.org/chrome-platform/c/c2dcb1b06053
- [v2,4/7] platform/chrome: cros_ec_proto: drop BUG_ON() in cros_ec_get_host_event()
https://git.kernel.org/chrome-platform/c/20a264c97bc8
- [v2,5/7] platform/chrome: cros_ec_i2c: drop BUG_ON() in cros_ec_pkt_xfer_i2c()
https://git.kernel.org/chrome-platform/c/8bff946c4199
- [v2,6/7] platform/chrome: cros_ec_spi: drop unneeded BUG_ON()
https://git.kernel.org/chrome-platform/c/ddec8e9e90ce
- [v2,7/7] platform/chrome: cros_ec_spi: drop BUG_ON() if `din` isn't large enough
https://git.kernel.org/chrome-platform/c/bbd43a37ec7a
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/7] platform/chrome: get rid of BUG_ON()
2022-05-13 4:41 [PATCH v2 0/7] platform/chrome: get rid of BUG_ON() Tzung-Bi Shih
` (7 preceding siblings ...)
2022-05-16 2:10 ` [PATCH v2 0/7] platform/chrome: get rid of BUG_ON() patchwork-bot+chrome-platform
@ 2022-05-18 5:20 ` patchwork-bot+chrome-platform
8 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+chrome-platform @ 2022-05-18 5:20 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform, linux-kernel
Hello:
This series was applied to chrome-platform/linux.git (for-next)
by Tzung-Bi Shih <tzungbi@kernel.org>:
On Fri, 13 May 2022 12:41:36 +0800 you wrote:
> The series gets rid of BUG_ON()s in drivers/platform/chrome/. Most of them
> can be replaced by returning proper return code.
>
> The 2nd patch makes callers of cros_ec_prepare_tx() to take care of the
> return code.
>
> The 3rd patch turns cros_ec_prepare_tx() to return error code if any.
>
> [...]
Here is the summary with links:
- [v2,1/7] platform/chrome: cros_ec_proto: drop unneeded BUG_ON() in prepare_packet()
https://git.kernel.org/chrome-platform/c/42701e7c0cd2
- [v2,2/7] platform/chrome: correct cros_ec_prepare_tx() usage
https://git.kernel.org/chrome-platform/c/71d3ae7fb640
- [v2,3/7] platform/chrome: cros_ec_proto: drop BUG_ON() in cros_ec_prepare_tx()
https://git.kernel.org/chrome-platform/c/c2dcb1b06053
- [v2,4/7] platform/chrome: cros_ec_proto: drop BUG_ON() in cros_ec_get_host_event()
https://git.kernel.org/chrome-platform/c/20a264c97bc8
- [v2,5/7] platform/chrome: cros_ec_i2c: drop BUG_ON() in cros_ec_pkt_xfer_i2c()
https://git.kernel.org/chrome-platform/c/8bff946c4199
- [v2,6/7] platform/chrome: cros_ec_spi: drop unneeded BUG_ON()
https://git.kernel.org/chrome-platform/c/ddec8e9e90ce
- [v2,7/7] platform/chrome: cros_ec_spi: drop BUG_ON() if `din` isn't large enough
https://git.kernel.org/chrome-platform/c/bbd43a37ec7a
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-05-18 5:20 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-13 4:41 [PATCH v2 0/7] platform/chrome: get rid of BUG_ON() Tzung-Bi Shih
2022-05-13 4:41 ` [PATCH v2 1/7] platform/chrome: cros_ec_proto: drop unneeded BUG_ON() in prepare_packet() Tzung-Bi Shih
2022-05-13 4:41 ` [PATCH v2 2/7] platform/chrome: correct cros_ec_prepare_tx() usage Tzung-Bi Shih
2022-05-13 4:41 ` [PATCH v2 3/7] platform/chrome: cros_ec_proto: drop BUG_ON() in cros_ec_prepare_tx() Tzung-Bi Shih
2022-05-13 4:41 ` [PATCH v2 4/7] platform/chrome: cros_ec_proto: drop BUG_ON() in cros_ec_get_host_event() Tzung-Bi Shih
2022-05-13 4:41 ` [PATCH v2 5/7] platform/chrome: cros_ec_i2c: drop BUG_ON() in cros_ec_pkt_xfer_i2c() Tzung-Bi Shih
2022-05-13 4:41 ` [PATCH v2 6/7] platform/chrome: cros_ec_spi: drop unneeded BUG_ON() Tzung-Bi Shih
2022-05-13 13:19 ` Guenter Roeck
2022-05-13 4:41 ` [PATCH v2 7/7] platform/chrome: cros_ec_spi: drop BUG_ON() if `din` isn't large enough Tzung-Bi Shih
2022-05-13 13:20 ` Guenter Roeck
2022-05-16 2:10 ` [PATCH v2 0/7] platform/chrome: get rid of BUG_ON() patchwork-bot+chrome-platform
2022-05-18 5:20 ` patchwork-bot+chrome-platform
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).