On Thu, Oct 24, 2019 at 11:03:20PM +0800, zhong jiang wrote:
> By reviewing the code, I find that there is an race between iterate
> the radix_tree and radix_tree_insert/delete. Because the former just
> access its slot in rcu protected period. but it fails to prevent the
> radix_tree from being changed.
Reviewed-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
The locking here now matches the locking in memfd_tag_pins() that
was changed in ef3038a573aa8bf2f3797b110f7244b55a0e519c (part of 4.20-rc1).
I didn't notice that I was fixing a bug when I changed the locking.
This bug has been present since 05f65b5c70909ef686f865f0a85406d74d75f70f
(part of 3.17) so backports will need to go further back. This code has
moved around a bit (mm/shmem.c) and the APIs have changed, so it will
take some effort.
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: zhong jiang <zhongjiang@xxxxxxxxxx>
> ---
> mm/memfd.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/mm/memfd.c b/mm/memfd.c
> index 2bb5e25..0b3fedc 100644
> --- a/mm/memfd.c
> +++ b/mm/memfd.c
> @@ -37,8 +37,8 @@ static void memfd_tag_pins(struct address_space *mapping)
>
> lru_add_drain();
> start = 0;
> - rcu_read_lock();
>
> + xa_lock_irq(&mapping->i_pages);
> radix_tree_for_each_slot(slot, &mapping->i_pages, &iter, start) {
> page = radix_tree_deref_slot(slot);
> if (!page || radix_tree_exception(page)) {
> @@ -47,18 +47,16 @@ static void memfd_tag_pins(struct address_space *mapping)
> continue;
> }
> } else if (page_count(page) - page_mapcount(page) > 1) {
> - xa_lock_irq(&mapping->i_pages);
> radix_tree_tag_set(&mapping->i_pages, iter.index,
> MEMFD_TAG_PINNED);
> - xa_unlock_irq(&mapping->i_pages);
> }
>
> if (need_resched()) {
> slot = radix_tree_iter_resume(slot, &iter);
> - cond_resched_rcu();
> + cond_resched_lock(&mapping->i_pages.xa_lock);
> }
> }
> - rcu_read_unlock();
> + xa_unlock_irq(&mapping->i_pages);
> }
>
> /*
> --
> 1.7.12.4
>
>
