linux-kernel - Re: [v2 PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm

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]

Date:   Tue, 27 Mar 2018 10:38:34 +0300
From:   Cyrill Gorcunov <gorcunov@...il.com>
To:     Yang Shi <yang.shi@...ux.alibaba.com>
Cc:     Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
        willy@...radead.org, adobriyan@...il.com, mhocko@...nel.org,
        mguzik@...hat.com, akpm@...ux-foundation.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [v2 PATCH] mm: introduce arg_lock to protect arg_start|end and
 env_start|end in mm_struct

On Mon, Mar 26, 2018 at 06:12:55PM -0400, Yang Shi wrote:
> > +	if (unlikely(arg_start > arg_end || env_start > env_end)) {
> > +		cond_resched();
> > +		goto retry;
> 
> Can't it trap into dead loop if the condition is always false?

Yes, unfortunately it can.

> > +	}
> > 
> > for reading these fields.
> > 
> > By the way, /proc/pid/ readers are serving as a canary who tells something
> > mm_mmap related problem is happening. On the other hand, it is sad that
> > such canary cannot be terminated by signal due to use of unkillable waits.
> > I wish we can use killable waits.
> 
> I already proposed patches (https://lkml.org/lkml/2018/2/26/1197) to do this
> a few weeks ago. In the review, akpm suggested mitigate the mmap_sem
> contention instead of using killable version workaround. Then the
> preliminary unmaping by section patches
> (https://lkml.org/lkml/2018/3/20/786) were proposed. In the discussion, we
> decided to eliminate the mmap_sem abuse, this is where the patch came from.