linux-kernel - Re: [PATCH] hugetlb: select PREEMPT_COUNT if HUGETLB_PAGE for in

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <YEnUeagd/CF1W8Db@dhcp22.suse.cz>
Date:   Thu, 11 Mar 2021 09:27:37 +0100
From:   Michal Hocko <mhocko@...e.com>
To:     Mike Kravetz <mike.kravetz@...cle.com>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        "Paul E . McKenney" <paulmck@...nel.org>,
        Shakeel Butt <shakeelb@...gle.com>, tglx@...utronix.de,
        john.ogness@...utronix.de, urezki@...il.com, ast@...com,
        Eric Dumazet <edumazet@...gle.com>,
        Mina Almasry <almasrymina@...gle.com>, peterz@...radead.org,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] hugetlb: select PREEMPT_COUNT if HUGETLB_PAGE for
 in_atomic use

On Thu 11-03-21 09:26:03, Michal Hocko wrote:
> On Wed 10-03-21 18:13:21, Mike Kravetz wrote:
> > put_page does not correctly handle all calling contexts for hugetlb
> > pages.  This was recently discussed in the threads [1] and [2].
> > 
> > free_huge_page is the routine called for the final put_page of huegtlb
> > pages.  Since at least the beginning of git history, free_huge_page has
> > acquired the hugetlb_lock to move the page to a free list and possibly
> > perform other processing. When this code was originally written, the
> > hugetlb_lock should have been made irq safe.
> > 
> > For many years, nobody noticed this situation until lockdep code caught
> > free_huge_page being called from irq context.  By this time, another
> > lock (hugetlb subpool) was also taken in the free_huge_page path.  In
> > addition, hugetlb cgroup code had been added which could hold
> > hugetlb_lock for a considerable period of time.  Because of this, commit
> > c77c0a8ac4c5 ("mm/hugetlb: defer freeing of huge pages if in non-task
> > context") was added to address the issue of free_huge_page being called
> > from irq context.  That commit hands off free_huge_page processing to a
> > workqueue if !in_task.
> > 
> > The !in_task check handles the case of being called from irq context.
> > However, it does not take into account the case when called with irqs
> > disabled as in [1].
> > 
> > To complicate matters, functionality has been added to hugetlb
> > such that free_huge_page may block/sleep in certain situations.  The
> > hugetlb_lock is of course dropped before potentially blocking.
> > 
> > One way to handle all calling contexts is to have free_huge_page always
> > send pages to the workqueue for processing.  This idea was briefly
> > discussed here [3], but has some undesirable side effects.
> 
> s@...esirable side effects@...esirable user visible side effects@
> 
> > Ideally, the hugetlb_lock should have been irq safe from the beginning
> > and any code added to the free_huge_page path should have taken this
> > into account.  However, this has not happened.  The code today does have
> > the ability to hand off requests to a workqueue.  It does this for calls
> > from irq context.  Changing the check in the code from !in_task to
> > in_atomic would handle the situations when called with irqs disabled.
> > However, it does not not handle the case when called with a spinlock
> > held.  This is needed because the code could block/sleep.
> > 
> > Select PREEMPT_COUNT if HUGETLB_PAGE is enabled so that in_atomic can be
> > used to detect all atomic contexts where sleeping is not possible.
> > 
> > [1] https://lore.kernel.org/linux-mm/000000000000f1c03b05bc43aadc@google.com/
> > [2] https://lore.kernel.org/linux-mm/YEjji9oAwHuZaZEt@dhcp22.suse.cz/
> > [3] https://lore.kernel.org/linux-mm/YDzaAWK41K4gD35V@dhcp22.suse.cz/
> > 
> > Suggested-by: Michal Hocko <mhocko@...e.com>
> > Signed-off-by: Mike Kravetz <mike.kravetz@...cle.com>
> 
> While not an ideal solution I believe this is the most straightforward
> one wrt to backporting to older kernels which are affected. I have a
> hope that a preemption model independent in_atomic() is going to become
> a norm. RCU is very much interested in the same thing as well. Now we
> have two core kernel users requiring this so hopefully this will make
> the case stronger.
> 
> That being said
> Acked-by: Michal Hocko <mhocko@...e.com>

Btw. we very likely want
Cc: stable

-- 
Michal Hocko
SUSE Labs