| 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 linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <571EFF78.2050603@ozlabs.ru>
Date: Tue, 26 Apr 2016 15:41:12 +1000
From: Alexey Kardashevskiy <aik@...abs.ru>
To: Yongji Xie <xyjxie@...ux.vnet.ibm.com>, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
linuxppc-dev@...ts.ozlabs.org, iommu@...ts.linux-foundation.org,
linux-doc@...r.kernel.org
Cc: alex.williamson@...hat.com, bhelgaas@...gle.com,
benh@...nel.crashing.org, paulus@...ba.org, mpe@...erman.id.au,
corbet@....net, warrier@...ux.vnet.ibm.com,
zhong@...ux.vnet.ibm.com, nikunj@...ux.vnet.ibm.com,
gwshan@...ux.vnet.ibm.com
Subject: Re: [RFC v6 04/10] PCI: Add support for enforcing all MMIO BARs to be
page aligned
On 04/18/2016 08:56 PM, Yongji Xie wrote:
> When vfio passthrough a PCI device of which MMIO BARs are
> smaller than PAGE_SIZE, guest will not handle the mmio
> accesses to the BARs which leads to mmio emulations in host.
>
> This is because vfio will not allow to passthrough one BAR's
> mmio page which may be shared with other BARs. Otherwise,
> there will be a backdoor that guest can use to access BARs
> of other guest.
>
> To solve this issue, this patch modifies resource_alignment
> to support syntax where multiple devices get the same
> alignment. So we can use something like
> "pci=resource_alignment=*:*:*.*:noresize" to enforce the
> alignment of all MMIO BARs to be at least PAGE_SIZE so that
> one BAR's mmio page would not be shared with other BARs.
>
> And we also define a macro PCIBIOS_MIN_ALIGNMENT to enable this
> automatically on PPC64 platform which can easily hit this issue
> because its PAGE_SIZE is 64KB.
>
> Signed-off-by: Yongji Xie <xyjxie@...ux.vnet.ibm.com>
> ---
> Documentation/kernel-parameters.txt | 2 ++
> arch/powerpc/include/asm/pci.h | 2 ++
> drivers/pci/pci.c | 64 +++++++++++++++++++++++++++++------
> 3 files changed, 57 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index d8b29ab..542be4a 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -2918,6 +2918,8 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> aligned memory resources.
> If <order of align> is not specified,
> PAGE_SIZE is used as alignment.
> + <domain>, <bus>, <slot> and <func> can be set to
> + "*" which means match all values.
> PCI-PCI bridge can be specified, if resource
> windows need to be expanded.
> noresize: Don't change the resources' sizes when
> diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
> index 6f8065a..78f230f 100644
> --- a/arch/powerpc/include/asm/pci.h
> +++ b/arch/powerpc/include/asm/pci.h
> @@ -30,6 +30,8 @@
> #define PCIBIOS_MIN_IO 0x1000
> #define PCIBIOS_MIN_MEM 0x10000000
>
> +#define PCIBIOS_MIN_ALIGNMENT PAGE_SIZE
> +
> struct pci_dev;
>
> /* Values for the `which' argument to sys_pciconfig_iobase syscall. */
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 7564ccc..0381c28 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4605,7 +4605,12 @@ static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
> int seg, bus, slot, func, align_order, count;
> resource_size_t align = 0;
> char *p;
> + bool invalid = false;
>
> +#ifdef PCIBIOS_MIN_ALIGNMENT
> + align = PCIBIOS_MIN_ALIGNMENT;
> + *resize = false;
> +#endif
> spin_lock(&resource_alignment_lock);
> p = resource_alignment_param;
> while (*p) {
> @@ -4622,16 +4627,49 @@ static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
> } else {
> align_order = -1;
> }
> - if (sscanf(p, "%x:%x:%x.%x%n",
> - &seg, &bus, &slot, &func, &count) != 4) {
I'd replace the above lines with:
char segstr[5] = "*", busstr[3] = "*";
char slotstr[3] = "*", funstr[2] = "*";
if (sscanf(p, "%4[^:]:%2[^:]:%2[^.].%1s%n",
&segstr, &busstr, &slotstr, &funcstr, &count) != 4) {
and add some wrapper like:
static bool glob_match_hex(char const *pat, int val)
{
char valstr[5]; /* 5 should be enough for PCI */
snprintf(valstr, sizeof(valstr) - 1, "%4x", val);
return glob_match(pat, valstr);
}
and then use glob_match_hex() (or make a wrapper like above on top of
fnmatch()), this would enable better mask handling.
If anyone finds this useful (which I am not sure about).
> + if (p[0] == '*' && p[1] == ':') {
> + seg = -1;
> + count = 1;
> + } else if (sscanf(p, "%x%n", &seg, &count) != 1 ||
> + p[count] != ':') {
> + invalid = true;
> + break;
> + }
> + p += count + 1;
> + if (*p == '*') {
> + bus = -1;
> + count = 1;
> + } else if (sscanf(p, "%x%n", &bus, &count) != 1) {
> + invalid = true;
> + break;
> + }
> + p += count;
> + if (*p == '.') {
> + slot = bus;
> + bus = seg;
> seg = 0;
> - if (sscanf(p, "%x:%x.%x%n",
> - &bus, &slot, &func, &count) != 3) {
> - /* Invalid format */
> - printk(KERN_ERR "PCI: Can't parse resource_alignment parameter: %s\n",
> - p);
> + p++;
> + } else if (*p == ':') {
> + p++;
> + if (p[0] == '*' && p[1] == '.') {
> + slot = -1;
> + count = 1;
> + } else if (sscanf(p, "%x%n", &slot, &count) != 1 ||
> + p[count] != '.') {
> + invalid = true;
> break;
> }
> + p += count + 1;
> + } else {
> + invalid = true;
> + break;
> + }
> + if (*p == '*') {
> + func = -1;
> + count = 1;
> + } else if (sscanf(p, "%x%n", &func, &count) != 1) {
> + invalid = true;
> + break;
> }
> p += count;
> if (!strncmp(p, ":noresize", 9)) {
> @@ -4639,10 +4677,10 @@ static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
> p += 9;
> } else
> *resize = true;
> - if (seg == pci_domain_nr(dev->bus) &&
> - bus == dev->bus->number &&
> - slot == PCI_SLOT(dev->devfn) &&
> - func == PCI_FUNC(dev->devfn)) {
> + if ((seg == pci_domain_nr(dev->bus) || seg == -1) &&
> + (bus == dev->bus->number || bus == -1) &&
> + (slot == PCI_SLOT(dev->devfn) || slot == -1) &&
> + (func == PCI_FUNC(dev->devfn) || func == -1)) {
> if (align_order == -1)
> align = PAGE_SIZE;
> else
> @@ -4652,10 +4690,14 @@ static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
> }
> if (*p != ';' && *p != ',') {
> /* End of param or invalid format */
> + invalid = true;
> break;
> }
> p++;
> }
> + if (invalid)
> + printk(KERN_ERR "PCI: Can't parse resource_alignment parameter:%s\n",
> + p);
> spin_unlock(&resource_alignment_lock);
> return align;
> }
>
--
Alexey
Powered by blists - more mailing lists