Skip to content

Commit 623c075

Browse files
Robert Richterdjbw
authored andcommitted
cxl/mbox: Fix Payload Length check for Get Log command
Commit 2aeaf66 introduced strict checking for variable length payload size validation. The payload length of received data must match the size of the requested data by the caller except for the case where the min_out value is set. The Get Log command does not have a header with a length field set. The Log size is determined by the Get Supported Logs command (CXL 3.0, 8.2.9.5.1). However, the actual size can be smaller and the number of valid bytes in the payload output must be determined reading the Payload Length field (CXL 3.0, Table 8-36, Note 2). Two issues arise: The command can successfully complete with a payload length of zero. And, the valid payload length must then also be consumed by the caller. Change cxl_xfer_log() to pass the number of payload bytes back to the caller to determine the number of log entries. Implement the payload handling as a special case where mbox_cmd->size_out is consulted when cxl_internal_send_cmd() returns -EIO. A WARN_ONCE() is added to check that -EIO is only returned in case of an unexpected output size. Logs can be bigger than the maximum payload length and multiple Get Log commands can be issued. If the received payload size is smaller than the maximum payload size we can assume all valid bytes have been fetched. Stop sending further Get Log commands then. On that occasion, change debug messages to also report the opcodes of supported commands. The variable payload commands GET_LSA and SET_LSA are not affected by this strict check: SET_LSA cannot be broken because SET_LSA does not return an output payload, and GET_LSA never expects short reads. Fixes: 2aeaf66 ("cxl/mbox: Add variable output size validation for internal commands") Signed-off-by: Robert Richter <[email protected]> Reviewed-by: Dave Jiang <[email protected]> Reviewed-by: Jonathan Cameron <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Dan Williams <[email protected]>
1 parent cbbd05d commit 623c075

File tree

1 file changed

+22
-3
lines changed

1 file changed

+22
-3
lines changed

drivers/cxl/core/mbox.c

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,12 @@ int cxl_internal_send_cmd(struct cxl_dev_state *cxlds,
170170
out_size = mbox_cmd->size_out;
171171
min_out = mbox_cmd->min_out;
172172
rc = cxlds->mbox_send(cxlds, mbox_cmd);
173+
/*
174+
* EIO is reserved for a payload size mismatch and mbox_send()
175+
* may not return this error.
176+
*/
177+
if (WARN_ONCE(rc == -EIO, "Bad return code: -EIO"))
178+
return -ENXIO;
173179
if (rc)
174180
return rc;
175181

@@ -550,9 +556,9 @@ int cxl_send_cmd(struct cxl_memdev *cxlmd, struct cxl_send_command __user *s)
550556
return 0;
551557
}
552558

553-
static int cxl_xfer_log(struct cxl_dev_state *cxlds, uuid_t *uuid, u32 size, u8 *out)
559+
static int cxl_xfer_log(struct cxl_dev_state *cxlds, uuid_t *uuid, u32 *size, u8 *out)
554560
{
555-
u32 remaining = size;
561+
u32 remaining = *size;
556562
u32 offset = 0;
557563

558564
while (remaining) {
@@ -576,6 +582,17 @@ static int cxl_xfer_log(struct cxl_dev_state *cxlds, uuid_t *uuid, u32 size, u8
576582
};
577583

578584
rc = cxl_internal_send_cmd(cxlds, &mbox_cmd);
585+
586+
/*
587+
* The output payload length that indicates the number
588+
* of valid bytes can be smaller than the Log buffer
589+
* size.
590+
*/
591+
if (rc == -EIO && mbox_cmd.size_out < xfer_size) {
592+
offset += mbox_cmd.size_out;
593+
break;
594+
}
595+
579596
if (rc < 0)
580597
return rc;
581598

@@ -584,6 +601,8 @@ static int cxl_xfer_log(struct cxl_dev_state *cxlds, uuid_t *uuid, u32 size, u8
584601
offset += xfer_size;
585602
}
586603

604+
*size = offset;
605+
587606
return 0;
588607
}
589608

@@ -695,7 +714,7 @@ int cxl_enumerate_cmds(struct cxl_dev_state *cxlds)
695714
goto out;
696715
}
697716

698-
rc = cxl_xfer_log(cxlds, &uuid, size, log);
717+
rc = cxl_xfer_log(cxlds, &uuid, &size, log);
699718
if (rc) {
700719
kvfree(log);
701720
goto out;

0 commit comments

Comments
 (0)