Hello Simon Glass,
The patch 10f746591fba: "cros_ec: Add vstore support" from Jan 16, 2021 (linux-next), leads to the following Smatch static checker warning:
drivers/misc/cros_ec_sandbox.c:543 process_cmd() error: buffer overflow 'ec->slot' 4 <= 31 drivers/misc/cros_ec_sandbox.c:556 process_cmd() error: buffer overflow 'ec->slot' 4 <= 31
drivers/misc/cros_ec_sandbox.c 521 len = sizeof(*resp); 522 break; 523 } 524 case EC_CMD_VSTORE_INFO: { 525 struct ec_response_vstore_info *resp = resp_data; 526 int i; 527 528 resp->slot_count = VSTORE_SLOT_COUNT;
There are two related defines. VSTORE_SLOT_COUNT (4) is the number of elements in ec->slot[].
529 resp->slot_locked = 0; 530 for (i = 0; i < VSTORE_SLOT_COUNT; i++) { 531 if (ec->slot[i].locked) 532 resp->slot_locked |= 1 << i; 533 } 534 len = sizeof(*resp); 535 break; 536 }; 537 case EC_CMD_VSTORE_WRITE: { 538 const struct ec_params_vstore_write *req = req_data; 539 struct vstore_slot *slot; 540 541 if (req->slot >= EC_VSTORE_SLOT_MAX) 542 return -EINVAL; --> 543 slot = &ec->slot[req->slot];
But here the check is for EC_VSTORE_SLOT_MAX (32) so Smatch thinks that 32 is more than 4 so this is an out of bounds. Should the limit be smaller or the array larger?
544 slot->locked = true; 545 memcpy(slot->data, req->data, EC_VSTORE_SLOT_SIZE); 546 len = 0; 547 break; 548 } 549 case EC_CMD_VSTORE_READ: { 550 const struct ec_params_vstore_read *req = req_data; 551 struct ec_response_vstore_read *resp = resp_data; 552 struct vstore_slot *slot; 553 554 if (req->slot >= EC_VSTORE_SLOT_MAX) 555 return -EINVAL; 556 slot = &ec->slot[req->slot];
Same.
557 memcpy(resp->data, slot->data, EC_VSTORE_SLOT_SIZE); 558 len = sizeof(*resp); 559 break; 560 } 561 case EC_CMD_PWM_GET_DUTY: { 562 const struct ec_params_pwm_get_duty *req = req_data;
regards, dan carpenter