linux-kernel - Re: [PATCH] Hugetlb: stop race when reading proc meminfo

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: <1215525062.7567.3.camel@localhost.localdomain>
Date:	Tue, 08 Jul 2008 09:51:02 -0400
From:	Eric Paris <eparis@...hat.com>
To:	Adam Litke <aglitke@...il.com>
Cc:	Nish Aravamudan <nish.aravamudan@...il.com>, wli@...omorphy.com,
	linux-kernel@...r.kernel.org, agl@...ibm.com,
	akpm@...ux-foundation.org
Subject: Re: [PATCH] Hugetlb: stop race when reading proc meminfo

On Thu, 2008-07-03 at 09:13 -0500, Adam Litke wrote:
> I agree with Nish on both of his points.  While it is slightly
> inconvenient for /proc/meminfo to occasionally display hugetlb pool
> counters that, together, don't make sense, it's worth living with for
> the two reasons he stated.  These counters are for informational
> purposes only and relying upon them will already get you into trouble
> in many other ways.  We also don't want to give any user the privilege
> to mount a denial of service attack on hugetlb applications.

I'm fine with this assessment.  Andrew feel free to throw this out of
-mm and forget about it forever.

-Eric


> 
> On Wed, Jul 2, 2008 at 5:51 PM, Nish Aravamudan
> <nish.aravamudan@...il.com> wrote:
> > On 6/19/08, Eric Paris <eparis@...hat.com> wrote:
> >> minor nit that hugetlb_report_{,node_}meminfo() does not lock the
> >>  reading of nr_huge_pages, free_huge_pages, and friends so this is not an
> >>  atomic set of information put into the buffer.  If /proc/meminfo is read
> >>  while the number of hugetlb pages is in flux it is possible to get
> >>  incorrect output such as:
> >>
> >>  HugePages_Total:     7
> >>  HugePages_Free:      8
> >>  HugePages_Rsvd:      0
> >>  Hugepagesize:     4096 kB
> >>
> >>  (test available at https://bugzilla.redhat.com/attachment.cgi?id=309864)
> >>
> >>  With the patch we beat on a number of boxes for hours with the above
> >>  test and saw no inconsistencies in the meminfo output.
> >>
> >>  Signed-off-by: Eric Paris <eparis@...hat.com>
> >
> > While I understand the spirit of this patch, I'm not sure it's really
> > necessary. We've never bothered locking the output in the proc file
> > before because the information is inherently racy. It only gives you a
> > view of what was...if any application tries to make a decision based
> > upon that information (unless it has global control of the system, or
> > is a testcase, arguably), then it is already racy -- that is, the
> > numbers you read at one point in time have no bearing on whether those
> > pages will actually be free/available, etc when you actually need
> > them.
> >
> > That being said, I don't feel to strongly about it. Just seems like it
> > might be unnecessary for an interface we know not to be fully accurate
> > to begin with, but maybe consistency is worth a bit of extra locking.
> >
> > That actually also leads me to wonder if maybe the locking is not
> > there currently to avoid letting readers of proc files from blocking
> > users of hugepages?
> >
> > Thanks,
> > Nish
> > --
> > 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/
> >
> 
> 
> 

--
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/