linux-kernel - Re: [PATCH v5] mm/ksm: introduce ksm

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: <6279cca3.1c69fb81.c4e50.581d@mx.google.com>
Date:   Tue, 10 May 2022 02:23:29 +0000
From:   CGEL <cgel.zte@...il.com>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     akpm@...ux-foundation.org, keescook@...omium.org,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, ran.xiaokai@....com.cn, wang.yong12@....com.cn,
        xu.xin16@....com.cn, yang.yang29@....com.cn,
        zhang.yunkai@....com.cn
Subject: Re: [PATCH v5] mm/ksm: introduce ksm_force for each process

On Mon, May 09, 2022 at 04:40:50PM +0100, Matthew Wilcox wrote:
> On Mon, May 09, 2022 at 06:57:33AM +0000, CGEL wrote:
> > On Sun, May 08, 2022 at 07:03:36PM +0100, Matthew Wilcox wrote:
> > > On Sun, May 08, 2022 at 09:27:10AM +0000, cgel.zte@...il.com wrote:
> > > > If ksm_force is set to 0, cancel the feature of ksm_force of this
> > > > process and unmerge those merged pages belonging to VMAs which is not
> > > > madvised as MADV_MERGEABLE of this process, but leave MADV_MERGEABLE
> > > > areas merged.
> > > 
> > > Is that actually a useful feature?  Otherwise, we could simply turn
> > > on/off the existing MMF_VM_MERGEABLE flag instead of introducing this
> > > new bool.
> > > 
> > I think this will be very useful for those apps which are very likely to
> > cause Same Pages in memory and users and operators are not willing to
> > modified the source codes for any reasons.
> 
> No, you misunderstand.  Is it useful to have the "force KSM off"
> functionality?  ie code which has been modified to allow KSM, but
> then overridden by an admin?
> 
Oh, I see what you mean. It should be mentioned that "force KSM off" is not
implemented for the current patch. In this patch, setting ksm_force to 0 just
restores the system to the default state (the state before patching)

> > Besides, simply turning of/off the existing MMF_VM_MERGEABLE flag may be
> > not feasible because madvise will also turn on the MMF_VM_MERGEABLE
> > flag.
> > 
> > I think the following suggestions is good, and I will resend a patch.
> > > > +Controlling KSM with procfs
> > > > +===========================
> > > > +
> > > > +KSM can also operate on anonymous areas of address space of those processes's
> > > > +knob ``/proc/<pid>/ksm_force`` is on, even if app codes doesn't call madvise()
> > > > +explicitly to advise specific areas as MADV_MERGEABLE.
> > > > +
> > > > +You can set ksm_force to 1 to force all anonymous and qualified VMAs of
> > > > +this process to be involved in KSM scanning. But It is effective only when the
> > > > +klob of ``/sys/kernel/mm/ksm/run`` is set as 1.
> > > 
> > > I think that last sentence doesn't really add any value.
> > > 
> > > > +	memset(buffer, 0, sizeof(buffer));
> > > > +	if (count > sizeof(buffer) - 1)
> > > > +		count = sizeof(buffer) - 1;
> > > > +	if (copy_from_user(buffer, buf, count)) {
> > > > +		err = -EFAULT;
> > > > +		goto out_return;
> > > 
> > > This feels a bit unnecessary.  Just 'return -EFAULT' here.
> > > 
> > > > +	}
> > > > +
> > > > +	err = kstrtoint(strstrip(buffer), 0, &force);
> > > > +
> > > > +	if (err)
> > > > +		goto out_return;
> > > 
> > > 'return err'
> > > 
> > > > +	if (force != 0 && force != 1) {
> > > > +		err = -EINVAL;
> > > > +		goto out_return;
> > > 
> > > 'return -EINVAL'
> > > 
> > > > +	}
> > > > +
> > > > +	task = get_proc_task(file_inode(file));
> > > > +	if (!task) {
> > > > +		err = -ESRCH;
> > > > +		goto out_return;
> > > 
> > > 'return -ESRCH'