public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Jarod Wilson <jarod@redhat.com>
Cc: linux-kernel@vger.kernel.org,
	Alexey Dobriyan <adobriyan@gmail.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Miklos Szeredi <miklos@szeredi.hu>, Zefan Li <lizefan@huawei.com>,
	Oleg Nesterov <oleg@redhat.com>
Subject: Re: [PATCH] fs/proc: allow larger /proc/<pid>/cmdline output
Date: Thu, 9 Apr 2015 21:12:00 -0700	[thread overview]
Message-ID: <20150409211200.78b8631e.akpm@linux-foundation.org> (raw)
In-Reply-To: <1428638342-6782-1-git-send-email-jarod@redhat.com>

On Thu,  9 Apr 2015 23:59:02 -0400 Jarod Wilson <jarod@redhat.com> wrote:

> There are people who run java. Sometimes, when it misbehaves, they try to
> figure out what's going on by dumping /proc/<pid>/cmdline, but the length
> of that output is currently capped by PAGE_SIZE (so x86_64's 4k, in most
> cases), and sometimes, java command lines are longer than 4k characters.
> 
> This change allows the user to request a larger max length, up to 4x
> PAGE_SIZE, but the default out-of-the-box setting should keep things the
> same as they ever were. The 4x maximum is somewhat arbitrary, but seemed
> like it should be more than enough, because really, if you have more than
> 16k characters on your command line, you're probably doing it wrong...
> 
> I've tested this lightly with non-java shell commands with really long
> parameters, and things are perfectly stable after several hundred
> iterations of exercising things on a system booted with both
> proc_pid_maxlen=8192 and 16384. I wouldn't call my testing exhaustive,
> and I may not have considered something that will blow up horribly here,
> so comments and clues welcomed.
> 
> Using single_open_size() looked less messy than giving proc_pid_cmdline()
> its own .start op that would allow multiple buffers.
> 
> Note: I've only added this extended sizing for /proc/<pid>/cmdline output,
> rather than for all /proc/<pid>/foo elements, thinking that nothing else
> should ever really be that long, but anything that is can simply switch
> from using the ONE() macro to the ONE_SIZE() macro.

Why have an upper limit at all?

> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -134,6 +134,30 @@ struct pid_entry {
>  	NOD(NAME, (S_IFREG|(MODE)), 			\
>  		NULL, &proc_single_file_operations,	\
>  		{ .proc_show = show } )
> +#define ONE_SIZE(NAME, MODE, show)				\
> +	NOD(NAME, (S_IFREG|(MODE)), 				\
> +		NULL, &proc_single_file_size_operations,	\
> +		{ .proc_show = show } )
> +
> +/*
> + * Its hideous, but some java gunk winds up with a cmdline that is longer
> + * than PAGE_SIZE, and some people want to be able to see all of it for
> + * debugging purposes. Allocate at least PAGE_SIZE, and allow the user to
> + * ask for up to PAGE_SIZE << 2 (4x) to help with that situation.
> + */
> +static unsigned long proc_pid_maxlen = PAGE_SIZE;
> +static int set_proc_pid_maxlen(char *str)
> +{
> +	if (!str)
> +		return 0;
> +
> +	proc_pid_maxlen = simple_strtoul(str, &str, 0);
> +	proc_pid_maxlen = max(PAGE_SIZE, proc_pid_maxlen);
> +	proc_pid_maxlen = min(PAGE_SIZE << 2, proc_pid_maxlen);
> +
> +	return 1;
> +}
> +__setup("proc_pid_maxlen=", set_proc_pid_maxlen);

This permits 4k-16k on x86 and 64k-256k on powerpc.  This makes the
kernel interface inconsistent across architectures, which is not good -
some applications will work OK on one arch but will fail when moved to
a different arch.

s/PAGE_SIZE/4096/g would fix that.


  reply	other threads:[~2015-04-10  4:08 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-10  3:59 [PATCH] fs/proc: allow larger /proc/<pid>/cmdline output Jarod Wilson
2015-04-10  4:12 ` Andrew Morton [this message]
2015-04-10 12:18   ` Jarod Wilson
2015-04-10 14:11     ` Alexey Dobriyan
2015-04-10 14:13       ` [PATCH try #3] proc: fix PAGE_SIZE limit of /proc/$PID/cmdline Alexey Dobriyan
2015-04-10 18:01         ` Jarod Wilson
2015-04-10 22:09           ` Alexey Dobriyan
2015-04-13 18:28             ` Jarod Wilson
2015-04-13 20:23               ` Andrew Morton
2015-04-10 20:45       ` [PATCH] fs/proc: allow larger /proc/<pid>/cmdline output Andrew Morton
2015-04-13 18:24         ` Jarod Wilson

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=20150409211200.78b8631e.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=adobriyan@gmail.com \
    --cc=ebiederm@xmission.com \
    --cc=jarod@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=miklos@szeredi.hu \
    --cc=oleg@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    /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