Commit bd4b65ee5e5f750da709ac10c70266876e515c23
Committed by
Anthony Liguori
1 parent
6f4cbd39
qemu/pci: check constant registers on load
Add "cmask" table of constant register masks: if a bit is not writeable and is set in cmask table, this bit is checked on load. An attempt to load an image that would change such a register causes load to fail. Use this table to make sure that load does not modify registers that guest can not change (directly or indirectly). Note: we can't just assume that read-only registers never change, because the guest could change a register indirectly. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Showing
2 changed files
with
30 additions
and
1 deletions
hw/pci.c
... | ... | @@ -152,13 +152,19 @@ void pci_device_save(PCIDevice *s, QEMUFile *f) |
152 | 152 | |
153 | 153 | int pci_device_load(PCIDevice *s, QEMUFile *f) |
154 | 154 | { |
155 | + uint8_t config[PCI_CONFIG_SPACE_SIZE]; | |
155 | 156 | uint32_t version_id; |
156 | 157 | int i; |
157 | 158 | |
158 | 159 | version_id = qemu_get_be32(f); |
159 | 160 | if (version_id > 2) |
160 | 161 | return -EINVAL; |
161 | - qemu_get_buffer(f, s->config, 256); | |
162 | + qemu_get_buffer(f, config, sizeof config); | |
163 | + for (i = 0; i < sizeof config; ++i) | |
164 | + if ((config[i] ^ s->config[i]) & s->cmask[i] & ~s->wmask[i]) | |
165 | + return -EINVAL; | |
166 | + memcpy(s->config, config, sizeof config); | |
167 | + | |
162 | 168 | pci_update_mappings(s); |
163 | 169 | |
164 | 170 | if (version_id >= 2) |
... | ... | @@ -254,6 +260,18 @@ static PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr) |
254 | 260 | return pci_find_bus(bus); |
255 | 261 | } |
256 | 262 | |
263 | +static void pci_init_cmask(PCIDevice *dev) | |
264 | +{ | |
265 | + pci_set_word(dev->cmask + PCI_VENDOR_ID, 0xffff); | |
266 | + pci_set_word(dev->cmask + PCI_DEVICE_ID, 0xffff); | |
267 | + dev->cmask[PCI_STATUS] = PCI_STATUS_CAP_LIST; | |
268 | + dev->cmask[PCI_REVISION_ID] = 0xff; | |
269 | + dev->cmask[PCI_CLASS_PROG] = 0xff; | |
270 | + pci_set_word(dev->cmask + PCI_CLASS_DEVICE, 0xffff); | |
271 | + dev->cmask[PCI_HEADER_TYPE] = 0xff; | |
272 | + dev->cmask[PCI_CAPABILITY_LIST] = 0xff; | |
273 | +} | |
274 | + | |
257 | 275 | static void pci_init_wmask(PCIDevice *dev) |
258 | 276 | { |
259 | 277 | int i; |
... | ... | @@ -286,6 +304,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, |
286 | 304 | pstrcpy(pci_dev->name, sizeof(pci_dev->name), name); |
287 | 305 | memset(pci_dev->irq_state, 0, sizeof(pci_dev->irq_state)); |
288 | 306 | pci_set_default_subsystem_id(pci_dev); |
307 | + pci_init_cmask(pci_dev); | |
289 | 308 | pci_init_wmask(pci_dev); |
290 | 309 | |
291 | 310 | if (!config_read) |
... | ... | @@ -385,6 +404,7 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, |
385 | 404 | } |
386 | 405 | *(uint32_t *)(pci_dev->config + addr) = cpu_to_le32(type); |
387 | 406 | *(uint32_t *)(pci_dev->wmask + addr) = cpu_to_le32(wmask); |
407 | + *(uint32_t *)(pci_dev->cmask + addr) = 0xffffffff; | |
388 | 408 | } |
389 | 409 | |
390 | 410 | static void pci_update_mappings(PCIDevice *d) |
... | ... | @@ -939,6 +959,8 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size) |
939 | 959 | memset(pdev->used + offset, 0xFF, size); |
940 | 960 | /* Make capability read-only by default */ |
941 | 961 | memset(pdev->wmask + offset, 0, size); |
962 | + /* Check capability by default */ | |
963 | + memset(pdev->cmask + offset, 0xFF, size); | |
942 | 964 | return offset; |
943 | 965 | } |
944 | 966 | |
... | ... | @@ -951,6 +973,8 @@ void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size) |
951 | 973 | pdev->config[prev] = pdev->config[offset + PCI_CAP_LIST_NEXT]; |
952 | 974 | /* Make capability writeable again */ |
953 | 975 | memset(pdev->wmask + offset, 0xff, size); |
976 | + /* Clear cmask as device-specific registers can't be checked */ | |
977 | + memset(pdev->cmask + offset, 0, size); | |
954 | 978 | memset(pdev->used + offset, 0, size); |
955 | 979 | |
956 | 980 | if (!pdev->config[PCI_CAPABILITY_LIST]) | ... | ... |
hw/pci.h
... | ... | @@ -101,6 +101,7 @@ typedef struct PCIIORegion { |
101 | 101 | #define PCI_COMMAND_MASTER 0x4 /* Enable bus master */ |
102 | 102 | #define PCI_STATUS 0x06 /* 16 bits */ |
103 | 103 | #define PCI_REVISION_ID 0x08 /* 8 bits */ |
104 | +#define PCI_CLASS_PROG 0x09 /* Reg. Level Programming Interface */ | |
104 | 105 | #define PCI_CLASS_DEVICE 0x0a /* Device class */ |
105 | 106 | #define PCI_CACHE_LINE_SIZE 0x0c /* 8 bits */ |
106 | 107 | #define PCI_LATENCY_TIMER 0x0d /* 8 bits */ |
... | ... | @@ -159,6 +160,10 @@ struct PCIDevice { |
159 | 160 | /* PCI config space */ |
160 | 161 | uint8_t config[PCI_CONFIG_SPACE_SIZE]; |
161 | 162 | |
163 | + /* Used to enable config checks on load. Note that writeable bits are | |
164 | + * never checked even if set in cmask. */ | |
165 | + uint8_t cmask[PCI_CONFIG_SPACE_SIZE]; | |
166 | + | |
162 | 167 | /* Used to implement R/W bytes */ |
163 | 168 | uint8_t wmask[PCI_CONFIG_SPACE_SIZE]; |
164 | 169 | ... | ... |