linux-kernel - Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices

Open Source and information security mailing list archives

Date:	Fri, 8 Jan 2016 11:15:37 +0800
From:	Wan ZongShun <mcuos.com@...il.com>
To:	Joerg Roedel <joro@...tes.org>
Cc:	Wan Zongshun <vincent.wan@....com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	iommu@...ts.linux-foundation.org, Ray Huang <ray.huang@....com>,
	Borislav Petkov <bp@...e.de>, ken.xue@....com, vw@...mu.org
Subject: Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices

2016-01-07 20:04 GMT+08:00 Joerg Roedel <joro@...tes.org>:
> On Tue, Jan 05, 2016 at 05:07:23AM -0500, Wan Zongshun wrote:
>> -static inline u16 get_device_id(struct device *dev)
>> +static inline int match_hid_uid(struct device *dev,
>> +                             struct acpihid_map_entry *entry)
>> +{
>> +     const char *hid, *uid;
>> +
>> +     hid = acpi_device_hid(ACPI_COMPANION(dev));
>> +     uid = acpi_device_uid(ACPI_COMPANION(dev));
>> +
>> +     if (!hid || !(*hid))
>> +             return -ENODEV;
>> +
>> +     if (!uid || !(*uid))
>> +             return strcmp(hid, entry->hid);
>> +
>> +     if (!(*entry->uid))
>> +             return strcmp(hid, entry->hid);
>> +
>> +     return -ENODEV;
>> +}
>> +
>> +static inline u16 get_pci_device_id(struct device *dev)
>>  {
>>       struct pci_dev *pdev = to_pci_dev(dev);
>>
>>       return PCI_DEVID(pdev->bus->number, pdev->devfn);
>>  }
>>
>> +static inline int get_acpihid_device_id(struct device *dev,
>> +                                     struct acpihid_map_entry **entry)
>> +{
>> +     struct acpihid_map_entry *p;
>> +
>> +     list_for_each_entry(p, &acpihid_map, list) {
>> +             if (!match_hid_uid(dev, p)) {
>> +                     if (entry)
>> +                             *entry = p;
>> +                     return p->devid;
>> +             }
>> +     }
>> +     return -EINVAL;
>> +}
>> +
>> +static inline u16 get_device_id(struct device *dev)
>> +{
>> +     if (dev_is_pci(dev))
>> +             return get_pci_device_id(dev);
>> +     else
>> +             return get_acpihid_device_id(dev, NULL);
>> +}
>
> This is not robust, get_acpihid_device_id() returns int and can return a
> negative value. This gets lost when converting it to u16 here. So either
> you add error handling for get_acpihid_device_id() in get_device_id() or
> you change get_device_id() to return int too and handle the error at the
> callers of get_device_id().

Joerg,

Please see the following function, since I judge this
'get_acpihid_device_id(dev, NULL) < 0'  in the front of
'get_device_id', so your concern should not exist. I have already
filtered the negative situation in check_device firstly, do you think
it is ok?

static bool check_device(struct device *dev)
{
        u16 devid;
......

        /* No PCI device */
        if (!dev_is_pci(dev) && (get_acpihid_device_id(dev, NULL) < 0))
                return false;

        devid = get_device_id(dev);

.....

        return true;
}

>
>
>         Joerg
>
> _______________________________________________
> iommu mailing list
> iommu@...ts.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

-- 
---
Vincent Wan(Zongshun)
www.mcuos.com

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