Re: [RFC PATCH kernel] vfio/spapr_tce: Get rid of possible infinite loop
From: Serhii Popovych <hidden>
Date: 2018-10-08 07:32:16
Alexey Kardashevskiy wrote:
quoted hunk ↗ jump to hunk
As a part of cleanup, the SPAPR TCE IOMMU subdriver releases preregistered memory. If there is a bug in memory release, the loop in tce_iommu_release() becomes infinite; this actually happened to me. This makes the loop finite and prints a warning on every failure to make the code more bug prone. Signed-off-by: Alexey Kardashevskiy <redacted> --- drivers/vfio/vfio_iommu_spapr_tce.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c index b1a8ab3..ece0651 100644 --- a/drivers/vfio/vfio_iommu_spapr_tce.c +++ b/drivers/vfio/vfio_iommu_spapr_tce.c@@ -371,6 +371,7 @@ static void tce_iommu_release(void *iommu_data) { struct tce_container *container = iommu_data; struct tce_iommu_group *tcegrp; + struct tce_iommu_prereg *tcemem, *tmtmp; long i; while (tce_groups_attached(container)) {@@ -393,13 +394,8 @@ static void tce_iommu_release(void *iommu_data) tce_iommu_free_table(container, tbl); } - while (!list_empty(&container->prereg_list)) { - struct tce_iommu_prereg *tcemem; - - tcemem = list_first_entry(&container->prereg_list, - struct tce_iommu_prereg, next); - WARN_ON_ONCE(tce_iommu_prereg_free(container, tcemem)); - } + list_for_each_entry_safe(tcemem, tmtmp, &container->prereg_list, next) + WARN_ON(tce_iommu_prereg_free(container, tcemem));
I'm not sure that tce_iommu_prereg_free() call under WARN_ON() is good idea because WARN_ON() is a preprocessor macro: if CONFIG_WARN=n is added by the analogy with CONFIG_BUG=n defining WARN_ON() as empty we will loose call to tce_iommu_prereg_free() leaking resources. There is no problem at the moment: WARN_ON() defined for PPC in arch/powerpc/include/asm/bug.h unconditionally. So your first version with intermediate variable looks better to me.
tce_iommu_disable(container); if (container->mm)
-- Thanks, Serhii
Attachments
- signature.asc [application/pgp-signature] 490 bytes