[PATCH] cros_ec: sandbox: Use correct value for number of slots

[PATCH] cros_ec: sandbox: Use correct value for number of slots

[Andrew Goodbody]

In the definition of struct ec_state the number of slots that are
created is VSTORE_SLOT_COUNT (==4) but the value of req->slot is
checked against EC_VSTORE_SLOT_MAX (==32) so this can lead to memory
access beyond that allocated.
Instead change the size check to use VSTORE_SLOT_COUNT to ensure it
matches what has actually been allocated.

This issue found by Smatch.

Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
---
 drivers/misc/cros_ec_sandbox.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/cros_ec_sandbox.c b/drivers/misc/cros_ec_sandbox.c
index 3ac690a3733..432b1fbb0c4 100644
--- a/drivers/misc/cros_ec_sandbox.c
+++ b/drivers/misc/cros_ec_sandbox.c
@@ -540,7 +540,7 @@ static int process_cmd(struct ec_state *ec,
 		const struct ec_params_vstore_write *req = req_data;
 		struct vstore_slot *slot;
 
-		if (req->slot >= EC_VSTORE_SLOT_MAX)
+		if (req->slot >= VSTORE_SLOT_COUNT)
 			return -EINVAL;
 		slot = &ec->slot[req->slot];
 		slot->locked = true;
@@ -553,7 +553,7 @@ static int process_cmd(struct ec_state *ec,
 		struct ec_response_vstore_read *resp = resp_data;
 		struct vstore_slot *slot;
 
-		if (req->slot >= EC_VSTORE_SLOT_MAX)
+		if (req->slot >= VSTORE_SLOT_COUNT)
 			return -EINVAL;
 		slot = &ec->slot[req->slot];
 		memcpy(resp->data, slot->data, EC_VSTORE_SLOT_SIZE);

---
base-commit: 7027b445cc0bfb86204ecb1f1fe596f5895048d9
change-id: 20250703-cros_ec_sandbox_fix-44fd2ed01ed7

Best regards,
-- 
Andrew Goodbody <andrew.goodbody@linaro.org>

Re: [PATCH] cros_ec: sandbox: Use correct value for number of slots

[Quentin Schulz]

Hi Andrew,

On 7/3/25 12:00 PM, Andrew Goodbody wrote:
> In the definition of struct ec_state the number of slots that are
> created is VSTORE_SLOT_COUNT (==4) but the value of req->slot is
> checked against EC_VSTORE_SLOT_MAX (==32) so this can lead to memory
> access beyond that allocated.
> Instead change the size check to use VSTORE_SLOT_COUNT to ensure it
> matches what has actually been allocated.
> 
> This issue found by Smatch.
> 
> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>

Seems reasonable to me:

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

We could have used "ARRAY_SIZE(ec->slot)" instead I believe but the rest 
of the code is already using VSTORE_SLOT_COUNT instead, so it makes 
sense to stay consistent.

Thanks!
Quentin

Re: [PATCH] cros_ec: sandbox: Use correct value for number of slots

[Tom Rini]

On Thu, 03 Jul 2025 11:00:33 +0100, Andrew Goodbody wrote:

> In the definition of struct ec_state the number of slots that are
> created is VSTORE_SLOT_COUNT (==4) but the value of req->slot is
> checked against EC_VSTORE_SLOT_MAX (==32) so this can lead to memory
> access beyond that allocated.
> Instead change the size check to use VSTORE_SLOT_COUNT to ensure it
> matches what has actually been allocated.
> 
> [...]

Applied to u-boot/master, thanks!

[1/1] cros_ec: sandbox: Use correct value for number of slots
      commit: bae3a34cb9e6b3ce6d01f68cdd34cdd45419f22f
-- 
Tom