On Thu, 7 Sept 2023 at 19:18, Stefan Hajnoczi <[email protected]> wrote:
>
> From: Jeuk Kim <[email protected]>
>
> This commit adds support for ufs logical unit.
> The LU handles processing for the SCSI command,
> unit descriptor query request.
>
> This commit enables the UFS device to process
> IO requests.
>
> Signed-off-by: Jeuk Kim <[email protected]>
> Reviewed-by: Stefan Hajnoczi <[email protected]>
> Message-id:
> beacc504376ab6a14b1a3830bb3c69382cf6aebc.1693980783.git.jeuk20....@gmail.com
> Signed-off-by: Stefan Hajnoczi <[email protected]>
Hi; Coverity points out a buffer overrun (CID 1519051) in
this commit:
> @@ -52,12 +76,18 @@ typedef struct UfsParams {
>
> typedef struct UfsHc {
> PCIDevice parent_obj;
> + UfsBus bus;
> MemoryRegion iomem;
> UfsReg reg;
> UfsParams params;
> uint32_t reg_size;
> UfsRequest *req_list;
>
> + UfsLu *lus[UFS_MAX_LUS];
The array lus[] is UFS_MAX_LUS in size...
> + UfsWLu *report_wlu;
> + UfsWLu *dev_wlu;
> + UfsWLu *boot_wlu;
> + UfsWLu *rpmb_wlu;
> DeviceDescriptor device_desc;
> GeometryDescriptor geometry_desc;
> Attributes attributes;
> @@ -716,9 +834,11 @@ static const RpmbUnitDescriptor rpmb_unit_desc = {
>
> static QueryRespCode ufs_read_unit_desc(UfsRequest *req)
> {
> + UfsHc *u = req->hc;
> uint8_t lun = req->req_upiu.qr.index;
>
> - if (lun != UFS_UPIU_RPMB_WLUN && lun > UFS_MAX_LUS) {
> + if (lun != UFS_UPIU_RPMB_WLUN &&
> + (lun > UFS_MAX_LUS || u->lus[lun] == NULL)) {
...but here the guard is "> UFS_MAX_LUS", which permits
lun == UFS_MAX_LUS, which will index off the end of the array here...
> trace_ufs_err_query_invalid_index(req->req_upiu.qr.opcode, lun);
> return UFS_QUERY_RESULT_INVALID_INDEX;
> }
> @@ -726,8 +846,8 @@ static QueryRespCode ufs_read_unit_desc(UfsRequest *req)
> if (lun == UFS_UPIU_RPMB_WLUN) {
> memcpy(&req->rsp_upiu.qr.data, &rpmb_unit_desc,
> rpmb_unit_desc.length);
> } else {
> - /* unit descriptor is not yet supported */
> - return UFS_QUERY_RESULT_INVALID_INDEX;
> + memcpy(&req->rsp_upiu.qr.data, &u->lus[lun]->unit_desc,
> + sizeof(u->lus[lun]->unit_desc));
...and here.
> }
>
> return UFS_QUERY_RESULT_SUCCESS;
Either the array needs to be larger, or the guard should be ">=".
thanks
-- PMM
