| Message ID | 20241101133917.27634-7-Jonathan.Cameron@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/cxl: Mailbox input parser hardening against invalid input. | expand |
On Fri, Nov 01, 2024 at 01:39:13PM +0000, Jonathan Cameron wrote: > Add a check that the requested offset + length does not go beyond the end > of the cel_log. > > Whilst the cci->cel_log is large enough to include all possible CEL > entries, the guest might still ask for entries beyond the end of it. > Move the comment to this new check rather than before the check on the > type of log requested. > > Reported-by: Esifiel <esifiel@gmail.com> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > hw/cxl/cxl-mailbox-utils.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > index 2aa7ffed84..5e571955b6 100644 > --- a/hw/cxl/cxl-mailbox-utils.c > +++ b/hw/cxl/cxl-mailbox-utils.c > @@ -937,24 +937,28 @@ static CXLRetCode cmd_logs_get_log(const struct cxl_cmd *cmd, > > get_log = (void *)payload_in; > > + if (get_log->length > cci->payload_max) { > + return CXL_MBOX_INVALID_INPUT; > + } > + > + if (!qemu_uuid_is_equal(&get_log->uuid, &cel_uuid)) { > + return CXL_MBOX_INVALID_LOG; > + } > + > /* > * CXL r3.1 Section 8.2.9.5.2: Get Log (Opcode 0401h) > * The device shall return Invalid Input if the Offset or Length > * fields attempt to access beyond the size of the log as reported by Get > - * Supported Logs. > + * Supported Log. > * > - * The CEL buffer is large enough to fit all commands in the emulation, so > - * the only possible failure would be if the mailbox itself isn't big > - * enough. > + * Only valid for there to be one entry per opcode, but the length + offset > + * may still be greater than that if the inputs are not valid and so access > + * beyond the end of cci->cel_log. > */ > - if (get_log->length > cci->payload_max) { > + if ((uint64_t)get_log->offset + get_log->length >= sizeof(cci->cel_log)) { > return CXL_MBOX_INVALID_INPUT; Oh. This patch actually addresses my concern in the previous patch. Maybe we can combine the changes in this patch and the previous one together. Other than that Reviewed-by: Fan Ni <fan.ni@samsung.com> > } > > - if (!qemu_uuid_is_equal(&get_log->uuid, &cel_uuid)) { > - return CXL_MBOX_INVALID_LOG; > - } > - > /* Store off everything to local variables so we can wipe out the payload */ > *len_out = get_log->length; > > -- > 2.43.0 >
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index 2aa7ffed84..5e571955b6 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -937,24 +937,28 @@ static CXLRetCode cmd_logs_get_log(const struct cxl_cmd *cmd, get_log = (void *)payload_in; + if (get_log->length > cci->payload_max) { + return CXL_MBOX_INVALID_INPUT; + } + + if (!qemu_uuid_is_equal(&get_log->uuid, &cel_uuid)) { + return CXL_MBOX_INVALID_LOG; + } + /* * CXL r3.1 Section 8.2.9.5.2: Get Log (Opcode 0401h) * The device shall return Invalid Input if the Offset or Length * fields attempt to access beyond the size of the log as reported by Get - * Supported Logs. + * Supported Log. * - * The CEL buffer is large enough to fit all commands in the emulation, so - * the only possible failure would be if the mailbox itself isn't big - * enough. + * Only valid for there to be one entry per opcode, but the length + offset + * may still be greater than that if the inputs are not valid and so access + * beyond the end of cci->cel_log. */ - if (get_log->length > cci->payload_max) { + if ((uint64_t)get_log->offset + get_log->length >= sizeof(cci->cel_log)) { return CXL_MBOX_INVALID_INPUT; } - if (!qemu_uuid_is_equal(&get_log->uuid, &cel_uuid)) { - return CXL_MBOX_INVALID_LOG; - } - /* Store off everything to local variables so we can wipe out the payload */ *len_out = get_log->length;
Add a check that the requested offset + length does not go beyond the end of the cel_log. Whilst the cci->cel_log is large enough to include all possible CEL entries, the guest might still ask for entries beyond the end of it. Move the comment to this new check rather than before the check on the type of log requested. Reported-by: Esifiel <esifiel@gmail.com> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> --- hw/cxl/cxl-mailbox-utils.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)