linux-kernel - Re: [PATCH v4 02/13] driver core: Set DMA ownership during driver bind/unbind

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 23 Dec 2021 15:23:54 +0800
From:   Lu Baolu <baolu.lu@...ux.intel.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     baolu.lu@...ux.intel.com, Joerg Roedel <joro@...tes.org>,
        Alex Williamson <alex.williamson@...hat.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Jason Gunthorpe <jgg@...dia.com>,
        Christoph Hellwig <hch@...radead.org>,
        Kevin Tian <kevin.tian@...el.com>,
        Ashok Raj <ashok.raj@...el.com>, Will Deacon <will@...nel.org>,
        Robin Murphy <robin.murphy@....com>,
        Dan Williams <dan.j.williams@...el.com>, rafael@...nel.org,
        Diana Craciun <diana.craciun@....nxp.com>,
        Cornelia Huck <cohuck@...hat.com>,
        Eric Auger <eric.auger@...hat.com>,
        Liu Yi L <yi.l.liu@...el.com>,
        Jacob jun Pan <jacob.jun.pan@...el.com>,
        Chaitanya Kulkarni <kch@...dia.com>,
        Stuart Yoder <stuyoder@...il.com>,
        Laurentiu Tudor <laurentiu.tudor@....com>,
        Thierry Reding <thierry.reding@...il.com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Li Yang <leoyang.li@....com>,
        Dmitry Osipenko <digetx@...il.com>,
        iommu@...ts.linux-foundation.org, linux-pci@...r.kernel.org,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 02/13] driver core: Set DMA ownership during driver
 bind/unbind

On 12/23/21 3:13 PM, Greg Kroah-Hartman wrote:
> On Thu, Dec 23, 2021 at 11:02:54AM +0800, Lu Baolu wrote:
>> Hi Greg,
>>
>> On 12/22/21 8:47 PM, Greg Kroah-Hartman wrote:
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +
>>>> +static void device_dma_cleanup(struct device *dev, struct device_driver *drv)
>>>> +{
>>>> +	if (!dev->bus->dma_configure)
>>>> +		return;
>>>> +
>>>> +	if (!drv->suppress_auto_claim_dma_owner)
>>>> +		iommu_device_release_dma_owner(dev, DMA_OWNER_DMA_API);
>>>> +}
>>>> +
>>>>    static int really_probe(struct device *dev, struct device_driver *drv)
>>>>    {
>>>>    	bool test_remove = IS_ENABLED(CONFIG_DEBUG_TEST_DRIVER_REMOVE) &&
>>>> @@ -574,11 +601,8 @@ static int really_probe(struct device *dev, struct device_driver *drv)
>>>>    	if (ret)
>>>>    		goto pinctrl_bind_failed;
>>>> -	if (dev->bus->dma_configure) {
>>>> -		ret = dev->bus->dma_configure(dev);
>>>> -		if (ret)
>>>> -			goto probe_failed;
>>>> -	}
>>>> +	if (device_dma_configure(dev, drv))
>>>> +		goto pinctrl_bind_failed;
>>> Are you sure you are jumping to the proper error path here?  It is not
>>> obvious why you changed this.
>> The error handling path in really_probe() seems a bit wrong. For
>> example,
>>
>>   572         /* If using pinctrl, bind pins now before probing */
>>   573         ret = pinctrl_bind_pins(dev);
>>   574         if (ret)
>>   575                 goto pinctrl_bind_failed;
>>
>> [...]
>>
>>   663 pinctrl_bind_failed:
>>   664         device_links_no_driver(dev);
>>   665         devres_release_all(dev);
>>   666         arch_teardown_dma_ops(dev);
>>   667         kfree(dev->dma_range_map);
>>   668         dev->dma_range_map = NULL;
>>   669         driver_sysfs_remove(dev);
>>               ^^^^^^^^^^^^^^^^^^^^^^^^^
>>   670         dev->driver = NULL;
>>   671         dev_set_drvdata(dev, NULL);
>>   672         if (dev->pm_domain && dev->pm_domain->dismiss)
>>   673                 dev->pm_domain->dismiss(dev);
>>   674         pm_runtime_reinit(dev);
>>   675         dev_pm_set_driver_flags(dev, 0);
>>   676 done:
>>   677         return ret;
>>
>> The driver_sysfs_remove() will be called even driver_sysfs_add() hasn't
>> been called yet. I can fix this in a separated patch if I didn't miss
>> anything.
> If this is a bug in the existing kernel, please submit it as a separate
> patch so that it can be properly backported to all affected kernels.
> Never bury it in an unrelated change that will never get sent to older
> kernels.

Sure! I will. Thank you!

Best regards,
baolu