Messages in this thread | | | Subject | Re: [RFC v1 6/8] mshv: command line option to skip devices in PV-IOMMU | From | Praveen Kumar <> | Date | Wed, 4 Aug 2021 12:33:54 +0530 |
| |
On 04-08-2021 03:26, Wei Liu wrote: >>> struct iommu_domain domain; >>> @@ -774,6 +784,41 @@ static struct iommu_device *hv_iommu_probe_device(struct device *dev) >>> if (!dev_is_pci(dev)) >>> return ERR_PTR(-ENODEV); >>> >>> + /* >>> + * Skip the PCI device specified in `pci_devs_to_skip`. This is a >>> + * temporary solution until we figure out a way to extract information >>> + * from the hypervisor what devices it is already using. >>> + */ >>> + if (pci_devs_to_skip && *pci_devs_to_skip) { >>> + int pos = 0; >>> + int parsed; >>> + int segment, bus, slot, func; >>> + struct pci_dev *pdev = to_pci_dev(dev); >>> + >>> + do { >>> + parsed = 0; >>> + >>> + sscanf(pci_devs_to_skip + pos, >>> + " (%x:%x:%x.%x) %n", >>> + &segment, &bus, &slot, &func, &parsed); >>> + >>> + if (parsed <= 0) >>> + break; >>> + >>> + if (pci_domain_nr(pdev->bus) == segment && >>> + pdev->bus->number == bus && >>> + PCI_SLOT(pdev->devfn) == slot && >>> + PCI_FUNC(pdev->devfn) == func) >>> + { >>> + dev_info(dev, "skipped by MSHV IOMMU\n"); >>> + return ERR_PTR(-ENODEV); >>> + } >>> + >>> + pos += parsed; >>> + >>> + } while (pci_devs_to_skip[pos]); >> >> Is there a possibility of pci_devs_to_skip + pos > sizeof(pci_devs_to_skip) >> and also a valid memory ? > > pos should point to the last parsed position. If parsing fails pos does > not get updated and the code breaks out of the loop. If parsing is > success pos should point to either the start of next element of '\0' > (end of string). To me this is good enough.
The point is, hypothetically the address to pci_devs_to_skip + pos can be valid address (later to '\0'), and thus there is a possibility, that parsing may not fail. Another, there is also a possibility of sscanf faulting accessing the illegal address, if pci_devs_to_skip[pos] turns out to be not NULL or valid address.
> >> I would recommend to have a check of size as well before accessing the >> array content, just to be safer accessing any memory. >> > > What check do you have in mind?
Something like, size_t len = strlen(pci_devs_to_skip); do {
len -= parsed; } while (len);
OR
do { ... pos += parsed; } while (pos < len);
Further, I'm also fine with the existing code, if you think this won't break and already been taken care. Thanks.
Regards,
~Praveen.
| |