linux-kernel - Re: [PATCH v14 09/22] vfio iommu type1: Add task structure to vfio

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 for Android: free password hash cracker in your pocket

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

Message-ID: <78bf03ef-8bdd-847d-9d49-c58f4efcf556@ozlabs.ru>
Date:   Thu, 17 Nov 2016 17:42:11 +1100
From:   Alexey Kardashevskiy <aik@...abs.ru>
To:     Alex Williamson <alex.williamson@...hat.com>
Cc:     Kirti Wankhede <kwankhede@...dia.com>, pbonzini@...hat.com,
        kraxel@...hat.com, cjia@...dia.com, qemu-devel@...gnu.org,
        kvm@...r.kernel.org, kevin.tian@...el.com, jike.song@...el.com,
        bjsdjshi@...ux.vnet.ibm.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v14 09/22] vfio iommu type1: Add task structure to
 vfio_dma

On 17/11/16 17:12, Alex Williamson wrote:
> On Thu, 17 Nov 2016 16:41:14 +1100
> Alexey Kardashevskiy <aik@...abs.ru> wrote:
> 
>> On 17/11/16 07:46, Kirti Wankhede wrote:
>>> Add task structure to vfio_dma structure. Task structure is used for:
>>> - During DMA_UNMAP, same task who mapped it or other task who shares same
>>> address space is allowed to unmap, otherwise unmap fails.
>>> QEMU maps few iova ranges initially, then fork threads and from the child
>>> thread calls DMA_UNMAP on previously mapped iova. Since child shares same
>>> address space, DMA_UNMAP is successful.
>>> - Avoid accessing struct mm while process is exiting by acquiring
>>> reference of task's mm during page accounting.
>>> - It is also used to get task mlock capability and rlimit for mlock.
>>>
>>> Signed-off-by: Kirti Wankhede <kwankhede@...dia.com>
>>> Signed-off-by: Neo Jia <cjia@...dia.com>
>>> Reviewed-by: Dong Jia Shi <bjsdjshi@...ux.vnet.ibm.com>  
>>
>>
>> I keep whinging that @mm should be referenced, not @current but you keep
>> referencing @current even if you only need @mm and you are not telling why
>> - and I am wondering what I am missing here? Something else will be used
>> from @task later, besides just @mm?
> 
> Yes, we reference @current from vfio_dma_do_map() and this is stored
> on the struct vfio_dma.  A reference to current is held because the
> external page pinning in vfio_pin_page_external() needs to test the
> capabilities of the task for CAP_IPC_LOCK to know whether locked memory

Ah, that's it - capable(CAP_IPC_LOCK) is checking @current, missed that.


> limits are in effect for the task even when it's not @current (ie. an
> asynchronous call from the vendor driver regardless of what task is
> currently running).  There are also various get_task_mm() taken
> temporarily when we're working with the mm of that task.  Do you spot
> any issues with this behavior? Thanks,

No, now I am fine, thanks!


-- 
Alexey