[PATCH] powerpc/iommu: Fix an RCU read lock leak in mm_iommu_is_devmem()

[PATCH] powerpc/iommu: Fix an RCU read lock leak in mm_iommu_is_devmem()

[Bart Van Assche]

mm_iommu_is_devmem() returns true from inside its RCU read-side
critical section without calling rcu_read_unlock(). Neither caller,
iommu_tce_xchg_no_kill() nor tce_page_is_contained(), drops the RCU
read lock, so every successful lookup leaks one RCU read lock. Fix this
by calling rcu_read_unlock() before returning true. The pointer to the
list element is not used after the RCU read lock is dropped.

Fixes: b5952f8125ae ("powerpc/mm/book3s64/iommu: fix some RCU-list locks")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 arch/powerpc/mm/book3s64/iommu_api.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/mm/book3s64/iommu_api.c b/arch/powerpc/mm/book3s64/iommu_api.c
index 60d00c9e7f19..e1e43573e8a8 100644
--- a/arch/powerpc/mm/book3s64/iommu_api.c
+++ b/arch/powerpc/mm/book3s64/iommu_api.c
@@ -371,6 +371,7 @@ bool mm_iommu_is_devmem(struct mm_struct *mm, unsigned long hpa,
 			 * and the caller needs to distinguish this situation.
 			 */
 			*size = min(1UL << pageshift, end - hpa);
+			rcu_read_unlock();
 			return true;
 		}
 	}

Re: [PATCH] powerpc/iommu: Fix an RCU read lock leak in mm_iommu_is_devmem()

[Markus Elfring]

…
> +++ b/arch/powerpc/mm/book3s64/iommu_api.c
> @@ -371,6 +371,7 @@ bool mm_iommu_is_devmem(struct mm_struct *mm, unsigned long hpa,
>  			 * and the caller needs to distinguish this situation.
>  			 */
>  			*size = min(1UL << pageshift, end - hpa);
> +			rcu_read_unlock();
>  			return true;
>  		}
>  	}

Under which circumstances would you become interested to apply a statement
like “guard(rcu)();”?
https://elixir.bootlin.com/linux/v7.3/source/include/linux/rcupdate.h#L1216

Regards,
Markus

Re: [PATCH] powerpc/iommu: Fix an RCU read lock leak in mm_iommu_is_devmem()

[Bart Van Assche]

On 10/26/26 1:27 PM, Markus Elfring wrote:
> Under which circumstances would you become interested to apply a statement
> like “guard(rcu)();”?

I will leave it to the PowerPC maintainers to share their preference.

Bart.

Re: [PATCH] powerpc/iommu: Fix an RCU read lock leak in mm_iommu_is_devmem()

[Ritesh Harjani]

Bart Van Assche <bvanassche@acm.org> writes:

> mm_iommu_is_devmem() returns true from inside its RCU read-side
> critical section without calling rcu_read_unlock(). Neither caller,
> iommu_tce_xchg_no_kill() nor tce_page_is_contained(), drops the RCU
> read lock, so every successful lookup leaks one RCU read lock. Fix this
> by calling rcu_read_unlock() before returning true. The pointer to the
> list element is not used after the RCU read lock is dropped.
>
> Fixes: b5952f8125ae ("powerpc/mm/book3s64/iommu: fix some RCU-list locks")

Thanks Bart, but this was caught as part of Clang's context analysis
work on powerpc.  It is already posted here [1]. Could you review that
instead?

[1]: https://lore.kernel.org/linuxppc-dev/04e4dafde56fe4b055fbd763dc0e119862073c6d.1790590014.git.ritesh.list@gmail.com/

-ritesh