linux-kernel - Re: [PACTH v1] mm, proc: Implement /proc/<pid>/totmaps

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:	Wed, 10 Aug 2016 19:37:19 +0200
From:	Jann Horn <jann@...jh.net>
To:	Sonny Rao <sonnyrao@...omium.org>
Cc:	Robert Foss <robert.foss@...labora.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Kees Cook <keescook@...omium.org>, viro@...iv.linux.org.uk,
	gorcunov@...nvz.org, John Stultz <john.stultz@...aro.org>,
	plaguedbypenguins@...il.com, Mateusz Guzik <mguzik@...hat.com>,
	adobriyan@...il.com, jdanis@...gle.com, calvinowens@...com,
	mhocko@...e.com, koct9i@...il.com, vbabka@...e.cz,
	n-horiguchi@...jp.nec.com, kirill.shutemov@...ux.intel.com,
	ldufour@...ux.vnet.ibm.com, Johannes Weiner <hannes@...xchg.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Ben Zhang <benzh@...omium.org>,
	Bryan Freed <bfreed@...omium.org>,
	Filipe Brandenburger <filbranden@...omium.org>
Subject: Re: [PACTH v1] mm, proc: Implement /proc/<pid>/totmaps

On Wed, Aug 10, 2016 at 10:23:53AM -0700, Sonny Rao wrote:
> On Tue, Aug 9, 2016 at 2:01 PM, Robert Foss <robert.foss@...labora.com> wrote:
> >
> >
> > On 2016-08-09 03:24 PM, Jann Horn wrote:
> >>
> >> On Tue, Aug 09, 2016 at 12:05:43PM -0400, robert.foss@...labora.com wrote:
> >>>
> >>> From: Sonny Rao <sonnyrao@...omium.org>
> >>>
> >>> This is based on earlier work by Thiago Goncales. It implements a new
> >>> per process proc file which summarizes the contents of the smaps file
> >>> but doesn't display any addresses.  It gives more detailed information
> >>> than statm like the PSS (proprotional set size).  It differs from the
> >>> original implementation in that it doesn't use the full blown set of
> >>> seq operations, uses a different termination condition, and doesn't
> >>> displayed "Locked" as that was broken on the original implemenation.
> >>>
> >>> This new proc file provides information faster than parsing the
> >>> potentially
> >>> huge smaps file.
> >>>
> >>> Signed-off-by: Sonny Rao <sonnyrao@...omium.org>
> >>>
> >>> Tested-by: Robert Foss <robert.foss@...labora.com>
> >>> Signed-off-by: Robert Foss <robert.foss@...labora.com>
> >>
> >>
> >>
> >>> +static int totmaps_proc_show(struct seq_file *m, void *data)
> >>> +{
> >>> +       struct proc_maps_private *priv = m->private;
> >>> +       struct mm_struct *mm;
> >>> +       struct vm_area_struct *vma;
> >>> +       struct mem_size_stats *mss_sum = priv->mss;
> >>> +
> >>> +       /* reference to priv->task already taken */
> >>> +       /* but need to get the mm here because */
> >>> +       /* task could be in the process of exiting */
> >>
> >>
> >> Can you please elaborate on this? My understanding here is that you
> >> intend for the caller to be able to repeatedly read the same totmaps
> >> file with pread() and still see updated information after the target
> >> process has called execve() and be able to detect process death
> >> (instead of simply seeing stale values). Is that accurate?
> >>
> >> I would prefer it if you could grab a reference to the mm_struct
> >> directly at open time.
> >
> >
> > Sonny, do you know more about the above comment?
> 
> I think right now the file gets re-opened every time, but the mode
> where the file is opened once and repeatedly read is interesting
> because it avoids having to open the file again and again.
> 
> I guess you could end up with a wierd situation where you don't read
> the entire contents of the file in open call to read() and you might
> get inconsistent data across the different statistics?

If the file is read in two chunks, totmaps_proc_show is only called
once. The patch specifies seq_read as read handler. Have a look at its
definition. As long as you don't read from the same seq file in
parallel or seek around in it, simple sequential reads will not
re-invoke the show() method for data that has already been formatted.
For partially consumed data, the kernel buffers the rest until someone
reads it or seeks to another offset.

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)