linux-kernel - Re: [PATCH v3 2/2] mm: attempt to batch free swap entries for zap_pte

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAGsJ_4zPDLL61DATTFd62sD-ZMd+4sFDaFCLsQatq=qAmOEnew@mail.gmail.com>
Date: Mon, 26 Aug 2024 11:52:10 +1200
From: Barry Song <21cnbao@...il.com>
To: Hugh Dickins <hughd@...gle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, chrisl@...nel.org, david@...hat.com, 
	justinjiang@...o.com, kaleshsingh@...gle.com, kasong@...cent.com, 
	linux-kernel@...r.kernel.org, linux-mm@...ck.org, ryan.roberts@....com, 
	v-songbaohua@...o.com, ying.huang@...el.com, yosryahmed@...gle.com
Subject: Re: [PATCH v3 2/2] mm: attempt to batch free swap entries for zap_pte_range()

On Mon, Aug 26, 2024 at 8:09 AM Hugh Dickins <hughd@...gle.com> wrote:
>
> On Fri, 16 Aug 2024, Barry Song wrote:
> > Subject: [PATCH] mm: check all swaps belong to same swap_cgroup in
> >  swap_pte_batch()
> >
> > Right now, it is possible two folios are contiguous in swap slots
> > but they don't belong to one memcg. In this case, even we return
> > a large nr, we can't really batch free all slots.
> >
> > Reported-by: Yosry Ahmed <yosryahmed@...gle.com>
> > Reported-by: Chris Li <chrisl@...nel.org>
> > Signed-off-by: Barry Song <v-songbaohua@...o.com>
> > ---
> >  mm/internal.h | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/internal.h b/mm/internal.h
> > index adbf8c88c9df..d1f1e221212d 100644
> > --- a/mm/internal.h
> > +++ b/mm/internal.h
> > @@ -15,6 +15,7 @@
> >  #include <linux/rmap.h>
> >  #include <linux/swap.h>
> >  #include <linux/swapops.h>
> > +#include <linux/swap_cgroup.h>
> >  #include <linux/tracepoint-defs.h>
> >
> >  /* Internal core VMA manipulation functions. */
> > @@ -275,18 +276,22 @@ static inline int swap_pte_batch(pte_t *start_ptep, int max_nr, pte_t pte)
> >  {
> >       pte_t expected_pte = pte_next_swp_offset(pte);
> >       const pte_t *end_ptep = start_ptep + max_nr;
> > +     swp_entry_t entry = pte_to_swp_entry(pte);
> >       pte_t *ptep = start_ptep + 1;
> > +     unsigned short cgroup_id;
> >
> >       VM_WARN_ON(max_nr < 1);
> >       VM_WARN_ON(!is_swap_pte(pte));
> > -     VM_WARN_ON(non_swap_entry(pte_to_swp_entry(pte)));
> > +     VM_WARN_ON(non_swap_entry(entry));
> >
> > +     cgroup_id = lookup_swap_cgroup_id(entry);
> >       while (ptep < end_ptep) {
> >               pte = ptep_get(ptep);
> >
> >               if (!pte_same(pte, expected_pte))
> >                       break;
> > -
> > +             if (lookup_swap_cgroup_id(pte_to_swp_entry(pte)) != cgroup_id)
> > +                     break;
> >               expected_pte = pte_next_swp_offset(expected_pte);
> >               ptep++;
> >       }
> > --
>
> [PATCH] mm: check all swaps belong to same swap_cgroup in swap_pte_batch() fix
>
> mm-unstable swap_pte_batch() adds a new usage of lookup_swap_cgroup_id(),
> which crashes if CONFIG_MEMCG kernel booted with "cgroup_disable=memory":
> it now needs a mem_cgroup_disabled() check.

sorry for the trouble.

>
> Fixes: 92b50df44566 ("mm: check all swaps belong to same swap_cgroup in swap_pte_batch()")
> Signed-off-by: Hugh Dickins <hughd@...gle.com>

Acked-by: Barry Song <baohua@...nel.org>

> ---
>  mm/swap_cgroup.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/mm/swap_cgroup.c b/mm/swap_cgroup.c
> index db6c4a26cf59..da1278f0563b 100644
> --- a/mm/swap_cgroup.c
> +++ b/mm/swap_cgroup.c
> @@ -161,6 +161,8 @@ unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id,
>   */
>  unsigned short lookup_swap_cgroup_id(swp_entry_t ent)
>  {
> +       if (mem_cgroup_disabled())
> +               return 0;
>         return lookup_swap_cgroup(ent, NULL)->id;
>  }
>
> --
> 2.35.3