lkml.org 
[lkml]   [2021]   [Aug]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC v1 6/8] mshv: command line option to skip devices in PV-IOMMU
    From
    Date
    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.

    \
     
     \ /
      Last update: 2021-08-04 09:05    [W:3.731 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site