[v2] hw/pci/pcie_doe: Check mailbox length for overflows

[PATCH v2] hw/pci/pcie_doe: Check mailbox length for overflows

Posted by alistair23@gmail.com 2 weeks, 5 days ago

From: Alistair Francis <alistair.francis@wdc.com>

It was possible that a guest could overflow the `doe_cap->write_mbox`
buffer by writing more then PCI_DOE_DW_SIZE_MAX dwords.
`doe_cap->write_mbox_len` would continue to increment and there were no
bounds checks on the length when offsetting into doe_cap->write_mbox.

This patch adds a check and reports a guest error if we would overflow.

On an overflow we also silenty discard the entire object as instructed
to do in the PCIe spec when the length specified in the header
(up to PCI_DOE_DW_SIZE_MAX dwords) doesn't match the length of the
object.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3679
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/pci/pcie_doe.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/hw/pci/pcie_doe.c b/hw/pci/pcie_doe.c
index 2210f86968..1bc2b45781 100644
--- a/hw/pci/pcie_doe.c
+++ b/hw/pci/pcie_doe.c
@@ -78,14 +78,21 @@ static bool pcie_doe_discovery(DOECap *doe_cap)
     return true;
 }
 
+static void pcie_doe_reset_write_mbox(DOECap *st)
+{
+    st->write_mbox_len = 0;
+
+    memset(st->write_mbox, 0, PCI_DOE_DW_SIZE_MAX * DWORD_BYTE);
+}
+
 static void pcie_doe_reset_mbox(DOECap *st)
 {
     st->read_mbox_idx = 0;
     st->read_mbox_len = 0;
-    st->write_mbox_len = 0;
 
     memset(st->read_mbox, 0, PCI_DOE_DW_SIZE_MAX * DWORD_BYTE);
-    memset(st->write_mbox, 0, PCI_DOE_DW_SIZE_MAX * DWORD_BYTE);
+
+    pcie_doe_reset_write_mbox(st);
 }
 
 void pcie_doe_init(PCIDevice *dev, DOECap *doe_cap, uint16_t offset,
@@ -356,8 +363,20 @@ void pcie_doe_write_config(DOECap *doe_cap,
         if (size != DWORD_BYTE) {
             return;
         }
-        doe_cap->write_mbox[doe_cap->write_mbox_len] = val;
-        doe_cap->write_mbox_len++;
+        if (doe_cap->write_mbox_len < PCI_DOE_DW_SIZE_MAX) {
+            doe_cap->write_mbox[doe_cap->write_mbox_len] = val;
+            doe_cap->write_mbox_len++;
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "Mailbox write length (%d) overflow\n",
+                          doe_cap->write_mbox_len);
+            /*
+             * Too much data has been written, it can't
+             * "match the Length indicated in DOE Data Object Header 2"
+             * so we drop the entire object.
+             */
+            pcie_doe_reset_write_mbox(doe_cap);
+        }
         break;
     case PCI_EXP_DOE_CAP:
         /* fallthrough */
-- 
2.54.0

Re: [PATCH v2] hw/pci/pcie_doe: Check mailbox length for overflows

Posted by Alistair Francis 2 weeks, 3 days ago

On Tue, Jul 7, 2026 at 12:08 PM <alistair23@gmail.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> It was possible that a guest could overflow the `doe_cap->write_mbox`
> buffer by writing more then PCI_DOE_DW_SIZE_MAX dwords.
> `doe_cap->write_mbox_len` would continue to increment and there were no
> bounds checks on the length when offsetting into doe_cap->write_mbox.
>
> This patch adds a check and reports a guest error if we would overflow.
>
> On an overflow we also silenty discard the entire object as instructed
> to do in the PCIe spec when the length specified in the header
> (up to PCI_DOE_DW_SIZE_MAX dwords) doesn't match the length of the
> object.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3679
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  hw/pci/pcie_doe.c | 27 +++++++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/hw/pci/pcie_doe.c b/hw/pci/pcie_doe.c
> index 2210f86968..1bc2b45781 100644
> --- a/hw/pci/pcie_doe.c
> +++ b/hw/pci/pcie_doe.c
> @@ -78,14 +78,21 @@ static bool pcie_doe_discovery(DOECap *doe_cap)
>      return true;
>  }
>
> +static void pcie_doe_reset_write_mbox(DOECap *st)
> +{
> +    st->write_mbox_len = 0;
> +
> +    memset(st->write_mbox, 0, PCI_DOE_DW_SIZE_MAX * DWORD_BYTE);
> +}
> +
>  static void pcie_doe_reset_mbox(DOECap *st)
>  {
>      st->read_mbox_idx = 0;
>      st->read_mbox_len = 0;
> -    st->write_mbox_len = 0;
>
>      memset(st->read_mbox, 0, PCI_DOE_DW_SIZE_MAX * DWORD_BYTE);
> -    memset(st->write_mbox, 0, PCI_DOE_DW_SIZE_MAX * DWORD_BYTE);
> +
> +    pcie_doe_reset_write_mbox(st);
>  }
>
>  void pcie_doe_init(PCIDevice *dev, DOECap *doe_cap, uint16_t offset,
> @@ -356,8 +363,20 @@ void pcie_doe_write_config(DOECap *doe_cap,
>          if (size != DWORD_BYTE) {
>              return;
>          }
> -        doe_cap->write_mbox[doe_cap->write_mbox_len] = val;
> -        doe_cap->write_mbox_len++;
> +        if (doe_cap->write_mbox_len < PCI_DOE_DW_SIZE_MAX) {
> +            doe_cap->write_mbox[doe_cap->write_mbox_len] = val;
> +            doe_cap->write_mbox_len++;
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "Mailbox write length (%d) overflow\n",
> +                          doe_cap->write_mbox_len);
> +            /*
> +             * Too much data has been written, it can't
> +             * "match the Length indicated in DOE Data Object Header 2"
> +             * so we drop the entire object.
> +             */
> +            pcie_doe_reset_write_mbox(doe_cap);
> +        }
>          break;
>      case PCI_EXP_DOE_CAP:
>          /* fallthrough */
> --
> 2.54.0
>

Re: [PATCH v2] hw/pci/pcie_doe: Check mailbox length for overflows

Posted by Tao Tang 2 weeks, 4 days ago

On 7/7/2026 10:07 AM, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
>
> It was possible that a guest could overflow the `doe_cap->write_mbox`
> buffer by writing more then PCI_DOE_DW_SIZE_MAX dwords.
> `doe_cap->write_mbox_len` would continue to increment and there were no
> bounds checks on the length when offsetting into doe_cap->write_mbox.
>
> This patch adds a check and reports a guest error if we would overflow.
>
> On an overflow we also silenty discard the entire object as instructed
> to do in the PCIe spec when the length specified in the header
> (up to PCI_DOE_DW_SIZE_MAX dwords) doesn't match the length of the
> object.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3679
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

Reviewed-by: Tao Tang <tangtao1634@phytium.com.cn>

Re: [PATCH v2] hw/pci/pcie_doe: Check mailbox length for overflows

Posted by Philippe Mathieu-Daudé 2 weeks, 5 days ago

On 7/7/26 04:07, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
> 
> It was possible that a guest could overflow the `doe_cap->write_mbox`
> buffer by writing more then PCI_DOE_DW_SIZE_MAX dwords.
> `doe_cap->write_mbox_len` would continue to increment and there were no
> bounds checks on the length when offsetting into doe_cap->write_mbox.
> 
> This patch adds a check and reports a guest error if we would overflow.
> 
> On an overflow we also silenty discard the entire object as instructed
> to do in the PCIe spec when the length specified in the header
> (up to PCI_DOE_DW_SIZE_MAX dwords) doesn't match the length of the
> object.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3679
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   hw/pci/pcie_doe.c | 27 +++++++++++++++++++++++----
>   1 file changed, 23 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>