linux-kernel - Re: [PATCH 13/13] maps#2: Add /proc/kpagemap interface

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

Message-Id: <1177014325.12599.166.camel@localhost.localdomain>
Date:	Thu, 19 Apr 2007 13:25:25 -0700
From:	Dave Hansen <hansendc@...ibm.com>
To:	Matt Mackall <mpm@...enic.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 13/13] maps#2: Add /proc/kpagemap interface

On Thu, 2007-04-19 at 15:02 -0500, Matt Mackall wrote:
> On Thu, Apr 19, 2007 at 12:06:38PM -0700, Dave Hansen wrote:
> > On Fri, 2007-04-06 at 17:03 -0500, Matt Mackall wrote:
> > > 
> > > +static ssize_t kpagemap_read(struct file *file, char __user *buf,
> > > +                            size_t count, loff_t *ppos)
> > > +{
> > ...
> > > +               for (; i < 2 * chunk / KPMSIZE; i += 2, pfn++) {
> > > +                       ppage = pfn_to_page(pfn);
> > > +                       if (!ppage) {
> > > +                               page[i] = 0;
> > > +                               page[i + 1] = 0;
> > > +                       } else {
> > > +                               page[i] = ppage->flags;
> > > +                               page[i + 1] = atomic_read(&ppage->_count);
> > > +                       }
> > > +               } 
> > 
> > I think this needs a pfn_valid() check for sparse/discontig systems.  I
> > think we'll oops if we have holes because we don't check pfn_valid()
> > inside of pfn_to_page().
> 
> Are there cases where pfn_valid is true but pfn_to_page doesn't work?
> In other words, do I still only need two cases?

pfn_valid() will at least guarantee that pfn_to_page() will give you
_some_ 'struct page' back.  

However, it isn't an absolute guarantee that there is actual physical
memory backing that struct page.  For your use here, it should be OK,
but it might be a bit confusing if anyone ever ends up with more entries
in that /proc file than they have pages of total memory.

But, this is a pretty low-level debugging mechanism, and I'm not sure
this is very easy to solve in an arch-independent way.

I'd probably just leave it looking something like this:

+               for (; i < 2 * chunk / KPMSIZE; i += 2, pfn++) {
+                       if (!pfn_valid(pfn)) {
+                              page[i] = 0;
+                              page[i + 1] = 0;
+                       } else {
+	                       ppage = pfn_to_page(pfn);
+                              page[i] = ppage->flags;
+                              page[i + 1] = atomic_read(&ppage->_count);
+                       }
+               } 

BTW, page->flags can be quite config-dependent on how stuff is stored in
there, especially with the zone, node, and section information encoded
in there.  Do you, perhaps, want to just spit out the bits that are
actual PageFoo() flags?

-- Dave

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/