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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <f7c540e2-57ff-62ea-da62-2595a3bf9141@nvidia.com>
Date:   Thu, 17 Nov 2016 11:53:09 +0530
From:   Kirti Wankhede <kwankhede@...dia.com>
To:     Alexey Kardashevskiy <aik@...abs.ru>, <alex.williamson@...hat.com>,
        <pbonzini@...hat.com>, <kraxel@...hat.com>, <cjia@...dia.com>
CC:     <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 11/17/2016 11:11 AM, Alexey Kardashevskiy 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?
> 
> 

Hey Alexey,

I updated briefly in commit description. Let me try to explain it again
in detail.

Its true we need mm, but we also need task structure for 2 reasons:
- Avoid accessing struct mm while process is exiting by acquiring
 reference of task's mm during page accounting.
If you see vfio_lock_acct(), where reference to mm is taken from task
structure, get_task_mm(task), to make sure that mm of this task is still
valid and task not in exiting process. If process is exiting, mm would
be NULL and we don't have to do page accounting.
This patch is to re-org and prepare the code for next patch, 10/22.
vfio_pin_pages()/ vfio_unpin_pages() for mediated devices would get
called from vendor driver. Those could be initiated by other process,
but for pin/unpin, these APIs should use the mm of the task who mapped
it. So in these calls we should check that we get the valid reference of
mm, that we would get from task structure.

- It is also used to get task mlock capability and rlimit for mlock.
These are again used for page accounting and page accounting should be
done with reference to the task who mapped the iova range. We get these
from task structure.

Thanks,
Kirti