lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 4 Aug 2021 12:33:54 +0530 From: Praveen Kumar <kumarpraveen@...ux.microsoft.com> To: Wei Liu <wei.liu@...nel.org> Cc: Linux on Hyper-V List <linux-hyperv@...r.kernel.org>, virtualization@...ts.linux-foundation.org, Linux Kernel List <linux-kernel@...r.kernel.org>, Michael Kelley <mikelley@...rosoft.com>, Vineeth Pillai <viremana@...ux.microsoft.com>, Sunil Muthuswamy <sunilmut@...rosoft.com>, Nuno Das Neves <nunodasneves@...ux.microsoft.com>, pasha.tatashin@...een.com, "K. Y. Srinivasan" <kys@...rosoft.com>, Haiyang Zhang <haiyangz@...rosoft.com>, Stephen Hemminger <sthemmin@...rosoft.com>, Dexuan Cui <decui@...rosoft.com>, Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>, "open list:IOMMU DRIVERS" <iommu@...ts.linux-foundation.org> Subject: Re: [RFC v1 6/8] mshv: command line option to skip devices in PV-IOMMU 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.
Powered by blists - more mailing lists