public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Edward Falk <efalk@google.com>
To: Jan Engelhardt <jengelh@linux01.gwdg.de>
Cc: linux-kernel@vger.kernel.org
Subject: Re: Proper /proc/pid/cmdline behavior when command line is corrupt?
Date: Fri, 08 Sep 2006 14:21:57 -0700	[thread overview]
Message-ID: <4501DEF5.8010300@google.com> (raw)
In-Reply-To: <Pine.LNX.4.61.0609082124350.30707@yvahk01.tjqt.qr>

Jan Engelhardt wrote:
> 
> http://www.x86-64.org/documentation/abi.pdf#search=%22argv%20envp%20x86%20ABI%22
> page 29 figure 3.9 lists the data block in question as "Information
> block, including argument strings, environment strings, auxiliary
> information", but does not specify it further, like how it is laid out.
> 
> What it does mention is "argument pointers" (aka argv[N]) and their
> exact position. In fact, right below the figure is the explanation:
> "Argument strings, environment strings, and the auxiliary information
> appear in no specific order within the information block and they need
> not be compactly allocated."

...

Thank you for sending me that document.  It seems that the bottom line 
is that the environment-follows-the-commandline assumption is *not* 
valid for future kernels, and may well not be valid for other 
architectures either.

I would suggest that for an application to overwrite the end of its own 
commandline buffer is undefined behavior.*

I'd also further suggest that the current implementation of 
proc_pid_cmdline() is essentially _guessing_ that the user has 
overwritten the end of the buffer and also guessing that the extra data 
can be found in the environment buffer.

Further, if a terminating nul still can't be found in the leading part 
of the environment buffer, proc_pid_cmdline() arbitrarily truncates the 
return value at the one-page mark with no attempt to insert a 
terminating nul.  It seems to me that if we accept that it's ok to 
arbitrarily truncate the return value, then a better choice of 
truncation point would be the end of the commandline buffer.

In addition, since the code is looking for missing data in the 
environment buffer, we can reasonably assume that the user has 
inadvertantly and hopelessly corrupted their own environment.  I submit 
that in this case, all bets are off and it doesn't really matter *what* 
we do at this point -- the results are undefined and can't possibly be 
correct.


> What that means for your future patch:
> 
> The way how the arg and env strings are laid out are, as far as I can 
> tell, defined in fs/binfmt_elf.c:create_elf_tables(). And 
> proc_pid_cmdline() depends on this layout, yes. However, since the 
> layout is not used anywhere outside the kernel (so says the PDF), there 
> should not be a problem. If you modify the layout, make sure it is 
> consistent within the kernel. It is unspecified for userspace, and a 
> user program accessing this area nonetheless is doing so at its own risk.

Thank you for pointing this out.  I'm not planning to change the layout, 
but perhaps a comment should be added to create_elf_tables() warning 
that proc_pid_cmdline() will need to be modified if the layout is ever 
modified.

Anyway; does anybody know why the original code was done this way, or of 
any applications that depend on that behavior?


> Hope this helps.

Immensely.  Thank you.

	-ed falk


*Actually, by the looks of things, for a process to write to its own 
commandline buffer *at all* is undefined behavior, since the spec makes 
no guarantees of the layout of the information block.

  reply	other threads:[~2006-09-08 21:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <mailman.3.1157626801.14788.linux-kernel-daily-digest@lists.us.dell.com>
2006-09-08  2:13 ` Proper /proc/pid/cmdline behavior when command line is corrupt? Edward Falk
2006-09-08  8:11   ` Jan Engelhardt
2006-09-08 17:16     ` Edward Falk
2006-09-08 20:05       ` Jan Engelhardt
2006-09-08 21:21         ` Edward Falk [this message]
2006-09-11  6:03           ` Jan Engelhardt

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=4501DEF5.8010300@google.com \
    --to=efalk@google.com \
    --cc=jengelh@linux01.gwdg.de \
    --cc=linux-kernel@vger.kernel.org \
    /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