| Message ID | 1675184289-267876-1-git-send-email-steven.sistare@oracle.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | fixes for virtual address update | expand |
On Tue, 31 Jan 2023 08:58:02 -0800 Steve Sistare <steven.sistare@oracle.com> wrote: > Changes in v8: > * updated group_leader comment in vfio_dma_do_map > * delete async arg from mm_lock_acct > * pass async=false to vfio_lock_acct in vfio_pin_page_external > * locked_vm becomes size_t > * improved commit message in "restore locked_vm" > * simplified flow in vfio_change_dma_owner > * rebase to v6.2-rc6 > > Steve Sistare (7): > vfio/type1: exclude mdevs from VFIO_UPDATE_VADDR > vfio/type1: prevent underflow of locked_vm via exec() > vfio/type1: track locked_vm per dma > vfio/type1: restore locked_vm > vfio/type1: revert "block on invalid vaddr" > vfio/type1: revert "implement notify callback" > vfio: revert "iommu driver notify callback" > > drivers/vfio/container.c | 5 - > drivers/vfio/vfio.h | 7 -- > drivers/vfio/vfio_iommu_type1.c | 248 ++++++++++++++++++---------------------- > include/uapi/linux/vfio.h | 15 ++- > 4 files changed, 120 insertions(+), 155 deletions(-) LGTM, I'll pause to give Jason a chance to review as well. Thanks for working on this! Alex
On Tue, Jan 31, 2023 at 08:58:02AM -0800, Steve Sistare wrote: > Fix bugs in the interfaces that allow the underlying memory object of an > iova range to be mapped in a new address space. They allow userland to > indefinitely block vfio mediated device kernel threads, and do not > propagate the locked_vm count to a new mm. Also fix a pre-existing bug > that allows locked_vm underflow. > > The fixes impose restrictions that eliminate waiting conditions, so > revert the dead code: > commit 898b9eaeb3fe ("vfio/type1: block on invalid vaddr") > commit 487ace134053 ("vfio/type1: implement notify callback") > commit ec5e32940cc9 ("vfio: iommu driver notify callback") I would still rather we delete this API. Something that doesn't work with iommufd, and doesn't work with mdevs doesn't seem like it should be in the kernel. But it is up to Alex, and the code looks fine: Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Jason
On Tue, 31 Jan 2023 08:58:02 -0800 Steve Sistare <steven.sistare@oracle.com> wrote: > Fix bugs in the interfaces that allow the underlying memory object of an > iova range to be mapped in a new address space. They allow userland to > indefinitely block vfio mediated device kernel threads, and do not > propagate the locked_vm count to a new mm. Also fix a pre-existing bug > that allows locked_vm underflow. > > The fixes impose restrictions that eliminate waiting conditions, so > revert the dead code: > commit 898b9eaeb3fe ("vfio/type1: block on invalid vaddr") > commit 487ace134053 ("vfio/type1: implement notify callback") > commit ec5e32940cc9 ("vfio: iommu driver notify callback") > > Changes in V2 (thanks Alex): > * do not allow group attach while vaddrs are invalid > * add patches to delete dead code > * add WARN_ON for never-should-happen conditions > * check for changed mm in unmap. > * check for vfio_lock_acct failure in remap > > Changes in V3 (ditto!): > * return errno at WARN_ON sites, and make it unique > * correctly check for dma task mm change > * change dma owner to current when vaddr is updated > * add Fixes to commit messages > * refactored new code in vfio_dma_do_map > > Changes in V4: > * misc cosmetic changes > > Changes in V5 (thanks Jason and Kevin): > * grab mm and use it for locked_vm accounting > * separate patches for underflow and restoring locked_vm > * account for reserved pages > * improve error messages > > Changes in V6: > * drop "count reserved pages" patch > * add "track locked_vm" patch > * grab current->mm not group_leader->mm > * simplify vfio_change_dma_owner > * fix commit messages > > Changes in v7: > * compare current->mm not group_leader->mm (missed one) > * misc cosmetic changes > > Changes in v8: > * updated group_leader comment in vfio_dma_do_map > * delete async arg from mm_lock_acct > * pass async=false to vfio_lock_acct in vfio_pin_page_external > * locked_vm becomes size_t > * improved commit message in "restore locked_vm" > * simplified flow in vfio_change_dma_owner > * rebase to v6.2-rc6 > > Steve Sistare (7): > vfio/type1: exclude mdevs from VFIO_UPDATE_VADDR > vfio/type1: prevent underflow of locked_vm via exec() > vfio/type1: track locked_vm per dma > vfio/type1: restore locked_vm > vfio/type1: revert "block on invalid vaddr" > vfio/type1: revert "implement notify callback" > vfio: revert "iommu driver notify callback" > > drivers/vfio/container.c | 5 - > drivers/vfio/vfio.h | 7 -- > drivers/vfio/vfio_iommu_type1.c | 248 ++++++++++++++++++---------------------- > include/uapi/linux/vfio.h | 15 ++- > 4 files changed, 120 insertions(+), 155 deletions(-) > Applied to vfio next branch for v6.3. Thanks, Alex
Fix bugs in the interfaces that allow the underlying memory object of an iova range to be mapped in a new address space. They allow userland to indefinitely block vfio mediated device kernel threads, and do not propagate the locked_vm count to a new mm. Also fix a pre-existing bug that allows locked_vm underflow. The fixes impose restrictions that eliminate waiting conditions, so revert the dead code: commit 898b9eaeb3fe ("vfio/type1: block on invalid vaddr") commit 487ace134053 ("vfio/type1: implement notify callback") commit ec5e32940cc9 ("vfio: iommu driver notify callback") Changes in V2 (thanks Alex): * do not allow group attach while vaddrs are invalid * add patches to delete dead code * add WARN_ON for never-should-happen conditions * check for changed mm in unmap. * check for vfio_lock_acct failure in remap Changes in V3 (ditto!): * return errno at WARN_ON sites, and make it unique * correctly check for dma task mm change * change dma owner to current when vaddr is updated * add Fixes to commit messages * refactored new code in vfio_dma_do_map Changes in V4: * misc cosmetic changes Changes in V5 (thanks Jason and Kevin): * grab mm and use it for locked_vm accounting * separate patches for underflow and restoring locked_vm * account for reserved pages * improve error messages Changes in V6: * drop "count reserved pages" patch * add "track locked_vm" patch * grab current->mm not group_leader->mm * simplify vfio_change_dma_owner * fix commit messages Changes in v7: * compare current->mm not group_leader->mm (missed one) * misc cosmetic changes Changes in v8: * updated group_leader comment in vfio_dma_do_map * delete async arg from mm_lock_acct * pass async=false to vfio_lock_acct in vfio_pin_page_external * locked_vm becomes size_t * improved commit message in "restore locked_vm" * simplified flow in vfio_change_dma_owner * rebase to v6.2-rc6 Steve Sistare (7): vfio/type1: exclude mdevs from VFIO_UPDATE_VADDR vfio/type1: prevent underflow of locked_vm via exec() vfio/type1: track locked_vm per dma vfio/type1: restore locked_vm vfio/type1: revert "block on invalid vaddr" vfio/type1: revert "implement notify callback" vfio: revert "iommu driver notify callback" drivers/vfio/container.c | 5 - drivers/vfio/vfio.h | 7 -- drivers/vfio/vfio_iommu_type1.c | 248 ++++++++++++++++++---------------------- include/uapi/linux/vfio.h | 15 ++- 4 files changed, 120 insertions(+), 155 deletions(-)