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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 6 Aug 2019 12:50:04 +0200
From:   Michal Hocko <mhocko@...nel.org>
To:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Masoud Sharbiani <msharbiani@...le.com>,
        Greg KH <gregkh@...uxfoundation.org>, hannes@...xchg.org,
        vdavydov.dev@...il.com, linux-mm@...ck.org,
        cgroups@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: Possible mem cgroup bug in kernels between 4.18.0 and 5.3-rc1.

On Tue 06-08-19 19:26:12, Tetsuo Handa wrote:
> On 2019/08/05 23:26, Michal Hocko wrote:
> > On Mon 05-08-19 23:00:12, Tetsuo Handa wrote:
> >> On 2019/08/05 20:44, Michal Hocko wrote:
> >>>> Allowing forced charge due to being unable to invoke memcg OOM killer
> >>>> will lead to global OOM situation, and just returning -ENOMEM will not
> >>>> solve memcg OOM situation.
> >>>
> >>> Returning -ENOMEM would effectivelly lead to triggering the oom killer
> >>> from the page fault bail out path. So effectively get us back to before
> >>> 29ef680ae7c21110. But it is true that this is riskier from the
> >>> observability POV when a) the OOM path wouldn't point to the culprit and
> >>> b) it would leak ENOMEM from g-u-p path.
> >>>
> >>
> >> Excuse me? But according to my experiment, below code showed flood of
> >> "Returning -ENOMEM" message instead of invoking the OOM killer.
> >> I didn't find it gets us back to before 29ef680ae7c21110...
> > 
> > You would need to declare OOM_ASYNC to return ENOMEM properly from the
> > charge (which is effectivelly a revert of 29ef680ae7c21110 for NOFS
> > allocations). Something like the following
> > 
> 
> OK. We need to set current->memcg_* before declaring something other than
> OOM_SUCCESS and OOM_FAILED... Well, it seems that returning -ENOMEM after
> setting current->memcg_* works as expected. What's wrong with your approach?

As I've said, and hoped you could pick up parts for your changelog for
the ENOMEM part, a) oom path is lost b) some paths will leak ENOMEM e.g.
g-u-p. So your patch to trigger the oom even for NOFS is a better
alternative I just found your ENOMEM note misleading and something that
could improve.
-- 
Michal Hocko
SUSE Labs

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ