linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: Potentially unbounded allocations in seq_read?
Date: Wed, 11 Dec 2013 18:00:07 +0000	[thread overview]
Message-ID: <20131211180007.GX10323@ZenIV.linux.org.uk> (raw)
In-Reply-To: <1386784112.6066.61.camel@tursulin-linux.isw.intel.com>

On Wed, Dec 11, 2013 at 05:48:32PM +0000, Tvrtko Ursulin wrote:
> On Wed, 2013-12-11 at 17:04 +0000, Tvrtko Ursulin wrote:
> > Hi all,
> > 
> > It seems that the buffer allocation in seq_read can double in size
> > indefinitely, at least I've seen that in practice with /proc/<pid>/smaps
> > (attempting to double m->size to 4M on a read of 1000 bytes). This
> > produces an ugly WARN_ON_ONCE, which should perhaps be avoided? (given
> > that it can be triggered by userspace at will)
> > 
> > From the top comment in seq_file.c one would think that it is a
> > fundamental limitation of the current code that everything which will be
> > read (even if in chunks) needs to be in the kernel side buffer at the
> > same time?
> 
> Oh-oh, seems that m->size is doubled on every read. So if app is reading
> with a buffer smaller than data available, it can do nine reads before
> it hits a >MAX_ORDER allocation. Not good. :)

Huh?  Is that from observation or from reading the code?  If it's the former,
I would really like to see details, if it's the latter... you are misreading
it.  m->size is doubled until it's large enough to hold ->show() output;
size argument of seq_read() has nothing to do with that.  Once the damn
thing is large enough, read() is served from it.  So are subsequent reads,
until you manage to eat all that had been generated.  Then the same buffer
is used for the next entry; again, no doubling unless that next entry is
even bigger and won't fit.  Doubling on each read(2) takes really strange
iterator to trigger and you'll need ->show() spewing bigger and bigger
entries.  Again - details, please...

  reply	other threads:[~2013-12-11 18:00 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-11 17:04 Potentially unbounded allocations in seq_read? Tvrtko Ursulin
2013-12-11 17:48 ` Tvrtko Ursulin
2013-12-11 18:00   ` Al Viro [this message]
2013-12-11 17:49 ` Al Viro
2013-12-11 17:59   ` Tvrtko Ursulin
2013-12-11 18:07     ` Al Viro
2013-12-12 13:40       ` Tvrtko Ursulin
2013-12-12 13:49         ` Al Viro
2013-12-12 13:59           ` Tvrtko Ursulin
2013-12-12 14:21             ` Al Viro
2013-12-12 14:52               ` Tvrtko Ursulin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131211180007.GX10323@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tvrtko.ursulin@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).