[bug report] mm: convert ksm_might_need_to_copy() to work on folios

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Matthew Wilcox (Oracle),

The patch 6faef19bdfcc: "mm: convert ksm_might_need_to_copy() to work
on folios" from Dec 11, 2023 (linux-next), leads to the following
Smatch static checker warning:

	mm/memory.c:4118 do_swap_page()
	error: 'folio' dereferencing possible ERR_PTR()

mm/memory.c
    3940                 /*
    3941                  * KSM sometimes has to copy on read faults, for example, if
    3942                  * page->index of !PageKSM() pages would be nonlinear inside the
    3943                  * anon VMA -- PageKSM() is lost on actual swapout.
    3944                  */
    3945                 folio = ksm_might_need_to_copy(folio, vma, vmf->address);
    3946                 if (unlikely(!folio)) {
    3947                         ret = VM_FAULT_OOM;
    3948                         goto out_page;
    3949                 } else if (unlikely(folio == ERR_PTR(-EHWPOISON))) {
                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This used to be page instead of folio


    3950                         ret = VM_FAULT_HWPOISON;
    3951                         goto out_page;

This goto will crash

    3952                 }
    3953                 page = folio_page(folio, 0);
    3954 
    3955                 /*
    3956                  * If we want to map a page that's in the swapcache writable, we
    3957                  * have to detect via the refcount if we're really the exclusive
    3958                  * owner. Try removing the extra reference from the local LRU
    3959                  * caches if required.
    3960                  */
    3961                 if ((vmf->flags & FAULT_FLAG_WRITE) && folio == swapcache &&
    3962                     !folio_test_ksm(folio) && !folio_test_lru(folio))
    3963                         lru_add_drain();
    3964         }
    3965 
    3966         folio_throttle_swaprate(folio, GFP_KERNEL);
    3967 
    3968         /*
    3969          * Back out if somebody else already faulted in this pte.
    3970          */
    3971         vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address,
    3972                         &vmf->ptl);
    3973         if (unlikely(!vmf->pte || !pte_same(ptep_get(vmf->pte), vmf->orig_pte)))
    3974                 goto out_nomap;
    3975 
    3976         if (unlikely(!folio_test_uptodate(folio))) {
    3977                 ret = VM_FAULT_SIGBUS;
    3978                 goto out_nomap;
    3979         }
    3980 
    3981         /*
    3982          * PG_anon_exclusive reuses PG_mappedtodisk for anon pages. A swap pte
    3983          * must never point at an anonymous page in the swapcache that is
    3984          * PG_anon_exclusive. Sanity check that this holds and especially, that
    3985          * no filesystem set PG_mappedtodisk on a page in the swapcache. Sanity
    3986          * check after taking the PT lock and making sure that nobody
    3987          * concurrently faulted in this page and set PG_anon_exclusive.
    3988          */
    3989         BUG_ON(!folio_test_anon(folio) && folio_test_mappedtodisk(folio));
    3990         BUG_ON(folio_test_anon(folio) && PageAnonExclusive(page));
    3991 
    3992         /*
    3993          * Check under PT lock (to protect against concurrent fork() sharing
    3994          * the swap entry concurrently) for certainly exclusive pages.
    3995          */
    3996         if (!folio_test_ksm(folio)) {
    3997                 exclusive = pte_swp_exclusive(vmf->orig_pte);
    3998                 if (folio != swapcache) {
    3999                         /*
    4000                          * We have a fresh page that is not exposed to the
    4001                          * swapcache -> certainly exclusive.
    4002                          */
    4003                         exclusive = true;
    4004                 } else if (exclusive && folio_test_writeback(folio) &&
    4005                           data_race(si->flags & SWP_STABLE_WRITES)) {
    4006                         /*
    4007                          * This is tricky: not all swap backends support
    4008                          * concurrent page modifications while under writeback.
    4009                          *
    4010                          * So if we stumble over such a page in the swapcache
    4011                          * we must not set the page exclusive, otherwise we can
    4012                          * map it writable without further checks and modify it
    4013                          * while still under writeback.
    4014                          *
    4015                          * For these problematic swap backends, simply drop the
    4016                          * exclusive marker: this is perfectly fine as we start
    4017                          * writeback only if we fully unmapped the page and
    4018                          * there are no unexpected references on the page after
    4019                          * unmapping succeeded. After fully unmapped, no
    4020                          * further GUP references (FOLL_GET and FOLL_PIN) can
    4021                          * appear, so dropping the exclusive marker and mapping
    4022                          * it only R/O is fine.
    4023                          */
    4024                         exclusive = false;
    4025                 }
    4026         }
    4027 
    4028         /*
    4029          * Some architectures may have to restore extra metadata to the page
    4030          * when reading from swap. This metadata may be indexed by swap entry
    4031          * so this must be called before swap_free().
    4032          */
    4033         arch_swap_restore(entry, folio);
    4034 
    4035         /*
    4036          * Remove the swap entry and conditionally try to free up the swapcache.
    4037          * We're already holding a reference on the page but haven't mapped it
    4038          * yet.
    4039          */
    4040         swap_free(entry);
    4041         if (should_try_to_free_swap(folio, vma, vmf->flags))
    4042                 folio_free_swap(folio);
    4043 
    4044         inc_mm_counter(vma->vm_mm, MM_ANONPAGES);
    4045         dec_mm_counter(vma->vm_mm, MM_SWAPENTS);
    4046         pte = mk_pte(page, vma->vm_page_prot);
    4047 
    4048         /*
    4049          * Same logic as in do_wp_page(); however, optimize for pages that are
    4050          * certainly not shared either because we just allocated them without
    4051          * exposing them to the swapcache or because the swap entry indicates
    4052          * exclusivity.
    4053          */
    4054         if (!folio_test_ksm(folio) &&
    4055             (exclusive || folio_ref_count(folio) == 1)) {
    4056                 if (vmf->flags & FAULT_FLAG_WRITE) {
    4057                         pte = maybe_mkwrite(pte_mkdirty(pte), vma);
    4058                         vmf->flags &= ~FAULT_FLAG_WRITE;
    4059                 }
    4060                 rmap_flags |= RMAP_EXCLUSIVE;
    4061         }
    4062         flush_icache_page(vma, page);
    4063         if (pte_swp_soft_dirty(vmf->orig_pte))
    4064                 pte = pte_mksoft_dirty(pte);
    4065         if (pte_swp_uffd_wp(vmf->orig_pte))
    4066                 pte = pte_mkuffd_wp(pte);
    4067         vmf->orig_pte = pte;
    4068 
    4069         /* ksm created a completely new copy */
    4070         if (unlikely(folio != swapcache && swapcache)) {
    4071                 folio_add_new_anon_rmap(folio, vma, vmf->address);
    4072                 folio_add_lru_vma(folio, vma);
    4073         } else {
    4074                 page_add_anon_rmap(page, vma, vmf->address, rmap_flags);
    4075         }
    4076 
    4077         VM_BUG_ON(!folio_test_anon(folio) ||
    4078                         (pte_write(pte) && !PageAnonExclusive(page)));
    4079         set_pte_at(vma->vm_mm, vmf->address, vmf->pte, pte);
    4080         arch_do_swap_page(vma->vm_mm, vma, vmf->address, pte, vmf->orig_pte);
    4081 
    4082         folio_unlock(folio);
    4083         if (folio != swapcache && swapcache) {
    4084                 /*
    4085                  * Hold the lock to avoid the swap entry to be reused
    4086                  * until we take the PT lock for the pte_same() check
    4087                  * (to avoid false positives from pte_same). For
    4088                  * further safety release the lock after the swap_free
    4089                  * so that the swap count won't change under a
    4090                  * parallel locked swapcache.
    4091                  */
    4092                 folio_unlock(swapcache);
    4093                 folio_put(swapcache);
    4094         }
    4095 
    4096         if (vmf->flags & FAULT_FLAG_WRITE) {
    4097                 ret |= do_wp_page(vmf);
    4098                 if (ret & VM_FAULT_ERROR)
    4099                         ret &= VM_FAULT_ERROR;
    4100                 goto out;
    4101         }
    4102 
    4103         /* No need to invalidate - it was non-present before */
    4104         update_mmu_cache_range(vmf, vma, vmf->address, vmf->pte, 1);
    4105 unlock:
    4106         if (vmf->pte)
    4107                 pte_unmap_unlock(vmf->pte, vmf->ptl);
    4108 out:
    4109         if (si)
    4110                 put_swap_device(si);
    4111         return ret;
    4112 out_nomap:
    4113         if (vmf->pte)
    4114                 pte_unmap_unlock(vmf->pte, vmf->ptl);
    4115 out_page:
    4116         folio_unlock(folio);
    4117 out_release:
--> 4118         folio_put(folio);
                 ^^^^^^^^^^^^^^^^

    4119         if (folio != swapcache && swapcache) {
    4120                 folio_unlock(swapcache);
    4121                 folio_put(swapcache);
    4122         }
    4123         if (si)
    4124                 put_swap_device(si);
    4125         return ret;
    4126 }

regards,
dan carpenter




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux