-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of Yonghua Huang
Sent: Friday, July 22, 2022 7:39 PM
To: acrn-dev@...
Cc: Huang, Yonghua <yonghua.huang@...>
Subject: [acrn-dev] [PATCH] hv: validate inputs in vpci_mmio_cfg_access

This function is registered as PCI MMIO configuration
access handler, which processes PCI configuration access
request from ACRN guest hence the inputs shall be validated.

Signed-off-by: Yonghua Huang <yonghua.huang@...>
---
hypervisor/dm/vpci/vpci.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/hypervisor/dm/vpci/vpci.c b/hypervisor/dm/vpci/vpci.c index
7c1f079701fb..861717a3130c 100644
--- a/hypervisor/dm/vpci/vpci.c
+++ b/hypervisor/dm/vpci/vpci.c
@@ -180,21 +180,26 @@ static int32_t vpci_mmio_cfg_access(struct
io_request *io_req, void *private_dat
uint32_t reg_num = (uint32_t)(address & 0xfffUL);
union pci_bdf bdf;

- /**
- * Enhanced Configuration Address Mapping
- * A[(20+n-1):20] Bus Number 1 ≤ n ≤ 8
- * A[19:15] Device Number
- * A[14:12] Function Number
- * A[11:8] Extended Register Number
- * A[7:2] Register Number
- * A[1:0] Along with size of the access, used to generate Byte Enables
- */
- bdf.value = (uint16_t)((address - pci_mmcofg_base) >> 12U);
+ if (pci_is_valid_access(reg_num, (uint32_t)mmio->size)) {
+ /**
+ * Enhanced Configuration Address Mapping
+ * A[(20+n-1):20] Bus Number 1 ≤ n ≤ 8
+ * A[19:15] Device Number
+ * A[14:12] Function Number
+ * A[11:8] Extended Register Number
+ * A[7:2] Register Number
+ * A[1:0] Along with size of the access, used to generate Byte
Enables
+ */
+ bdf.value = (uint16_t)((address - pci_mmcofg_base) >> 12U);

- if (mmio->direction == ACRN_IOREQ_DIR_READ) {
- ret = vpci_read_cfg(vpci, bdf, reg_num, (uint32_t)mmio->size,
(uint32_t *)&mmio->value);
+ if (mmio->direction == ACRN_IOREQ_DIR_READ) {
+ ret = vpci_read_cfg(vpci, bdf, reg_num,
(uint32_t)mmio->size, (uint32_t *)&mmio->value);
+ } else {
+ ret = vpci_write_cfg(vpci, bdf, reg_num,
(uint32_t)mmio->size, (uint32_t)mmio->value);
+ }
} else {
- ret = vpci_write_cfg(vpci, bdf, reg_num, (uint32_t)mmio->size,
(uint32_t)mmio->value);
+ pr_err("%s, invalid PCI config access, offset:0x%x, size:%d.\n",
+ __func__, reg_num, (uint32_t)mmio->size);

}

return ret;
--
2.25.1

-----Original Message-----
From: Dong, Eddie <eddie.dong@...>
Sent: Tuesday, July 26, 2022 08:34
To: acrn-dev@...
Cc: Huang, Yonghua <yonghua.huang@...>
Subject: RE: [acrn-dev] [PATCH] hv: validate inputs in vpci_mmio_cfg_access

-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of Yonghua Huang
Sent: Friday, July 22, 2022 7:39 PM
To: acrn-dev@...
Cc: Huang, Yonghua <yonghua.huang@...>
Subject: [acrn-dev] [PATCH] hv: validate inputs in vpci_mmio_cfg_access

This function is registered as PCI MMIO configuration
access handler, which processes PCI configuration access
request from ACRN guest hence the inputs shall be validated.

Signed-off-by: Yonghua Huang <yonghua.huang@...>
---
hypervisor/dm/vpci/vpci.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/hypervisor/dm/vpci/vpci.c b/hypervisor/dm/vpci/vpci.c index
7c1f079701fb..861717a3130c 100644
--- a/hypervisor/dm/vpci/vpci.c
+++ b/hypervisor/dm/vpci/vpci.c
@@ -180,21 +180,26 @@ static int32_t vpci_mmio_cfg_access(struct
io_request *io_req, void *private_dat
uint32_t reg_num = (uint32_t)(address & 0xfffUL);
union pci_bdf bdf;

- /**
- * Enhanced Configuration Address Mapping
- * A[(20+n-1):20] Bus Number 1 ≤ n ≤ 8
- * A[19:15] Device Number
- * A[14:12] Function Number
- * A[11:8] Extended Register Number
- * A[7:2] Register Number
- * A[1:0] Along with size of the access, used to generate Byte Enables
- */
- bdf.value = (uint16_t)((address - pci_mmcofg_base) >> 12U);
+ if (pci_is_valid_access(reg_num, (uint32_t)mmio->size)) {
+ /**
+ * Enhanced Configuration Address Mapping
+ * A[(20+n-1):20] Bus Number 1 ≤ n ≤ 8
+ * A[19:15] Device Number
+ * A[14:12] Function Number
+ * A[11:8] Extended Register Number
+ * A[7:2] Register Number
+ * A[1:0] Along with size of the access, used to generate Byte
Enables
+ */
+ bdf.value = (uint16_t)((address - pci_mmcofg_base) >> 12U);

- if (mmio->direction == ACRN_IOREQ_DIR_READ) {
- ret = vpci_read_cfg(vpci, bdf, reg_num, (uint32_t)mmio->size,
(uint32_t *)&mmio->value);
+ if (mmio->direction == ACRN_IOREQ_DIR_READ) {
+ ret = vpci_read_cfg(vpci, bdf, reg_num,
(uint32_t)mmio->size, (uint32_t *)&mmio->value);
+ } else {
+ ret = vpci_write_cfg(vpci, bdf, reg_num,
(uint32_t)mmio->size, (uint32_t)mmio->value);
+ }
} else {
- ret = vpci_write_cfg(vpci, bdf, reg_num, (uint32_t)mmio->size,
(uint32_t)mmio->value);
+ pr_err("%s, invalid PCI config access, offset:0x%x, size:%d.\n",
+ __func__, reg_num, (uint32_t)mmio->size);

So guest unaligned read get undefined result, while write is ignored. Is this
what you want?

}

return ret;
--
2.25.1

-----Original Message-----
From: Huang, Yonghua <yonghua.huang@...>
Sent: Monday, July 25, 2022 6:31 PM
To: Dong, Eddie <eddie.dong@...>; acrn-dev@...
Cc: Huang, Yonghua <yonghua.huang@...>
Subject: RE: [acrn-dev] [PATCH] hv: validate inputs in vpci_mmio_cfg_access

Hi Eddie,

-----Original Message-----
From: Dong, Eddie <eddie.dong@...>
Sent: Tuesday, July 26, 2022 08:34
To: acrn-dev@...
Cc: Huang, Yonghua <yonghua.huang@...>
Subject: RE: [acrn-dev] [PATCH] hv: validate inputs in
vpci_mmio_cfg_access

-----Original Message-----
From: acrn-dev@...
<acrn-dev@...> On Behalf Of Yonghua Huang
Sent: Friday, July 22, 2022 7:39 PM
To: acrn-dev@...
Cc: Huang, Yonghua <yonghua.huang@...>
Subject: [acrn-dev] [PATCH] hv: validate inputs in
vpci_mmio_cfg_access

This function is registered as PCI MMIO configuration
access handler, which processes PCI configuration access
request from ACRN guest hence the inputs shall be validated.

Signed-off-by: Yonghua Huang <yonghua.huang@...>
---
hypervisor/dm/vpci/vpci.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/hypervisor/dm/vpci/vpci.c b/hypervisor/dm/vpci/vpci.c
index 7c1f079701fb..861717a3130c 100644
--- a/hypervisor/dm/vpci/vpci.c
+++ b/hypervisor/dm/vpci/vpci.c
@@ -180,21 +180,26 @@ static int32_t vpci_mmio_cfg_access(struct
io_request *io_req, void *private_dat
uint32_t reg_num = (uint32_t)(address & 0xfffUL);
union pci_bdf bdf;

- /**
- * Enhanced Configuration Address Mapping
- * A[(20+n-1):20] Bus Number 1 ≤ n ≤ 8
- * A[19:15] Device Number
- * A[14:12] Function Number
- * A[11:8] Extended Register Number
- * A[7:2] Register Number
- * A[1:0] Along with size of the access, used to generate Byte Enables
- */
- bdf.value = (uint16_t)((address - pci_mmcofg_base) >> 12U);
+ if (pci_is_valid_access(reg_num, (uint32_t)mmio->size)) {
+ /**
+ * Enhanced Configuration Address Mapping
+ * A[(20+n-1):20] Bus Number 1 ≤ n ≤ 8
+ * A[19:15] Device Number
+ * A[14:12] Function Number
+ * A[11:8] Extended Register Number
+ * A[7:2] Register Number
+ * A[1:0] Along with size of the access, used to generate Byte
Enables
+ */
+ bdf.value = (uint16_t)((address - pci_mmcofg_base) >> 12U);

- if (mmio->direction == ACRN_IOREQ_DIR_READ) {
- ret = vpci_read_cfg(vpci, bdf, reg_num, (uint32_t)mmio->size,
(uint32_t *)&mmio->value);
+ if (mmio->direction == ACRN_IOREQ_DIR_READ) {
+ ret = vpci_read_cfg(vpci, bdf, reg_num,
(uint32_t)mmio->size, (uint32_t *)&mmio->value);
+ } else {
+ ret = vpci_write_cfg(vpci, bdf, reg_num,
(uint32_t)mmio->size, (uint32_t)mmio->value);
+ }
} else {
- ret = vpci_write_cfg(vpci, bdf, reg_num, (uint32_t)mmio->size,
(uint32_t)mmio->value);
+ pr_err("%s, invalid PCI config access, offset:0x%x, size:%d.\n",
+ __func__, reg_num, (uint32_t)mmio->size);

So guest unaligned read get undefined result, while write is ignored.
Is this what you want?

Can we follow the same rules in: vpci_pio_cfgdata_read/write() for this case?
-- Ignore for write and return (~0U, all '1's) for read

Thanks.

}

return ret;
--
2.25.1