All of lore.kernel.org
 help / color / mirror / Atom feed
From: Serge Hallyn <serge.hallyn@canonical.com>
To: Artem Bityutskiy <dedekind1@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Kees Cook <keescook@chromium.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] proc: pid/status: show all supplementary groups
Date: Fri, 9 Nov 2012 09:28:54 -0600	[thread overview]
Message-ID: <20121109152854.GA6778@sergelap> (raw)
In-Reply-To: <1352467863-1371-1-git-send-email-dedekind1@gmail.com>

Quoting Artem Bityutskiy (dedekind1@gmail.com):
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> 
> We display a list of supplementary group for each process in the
> /proc/<pid>/status. However, we show only the first 32 groups, not all of them.
> 
> Although this is rare, but sometimes processes do have more than 32
> supplementary groups, and this kernel limitation breaks user-space apps
> that rely on the group list in /proc/<pid>/status.
> 
> Number 32 comes from the internal NGROUPS_SMALL macro which defines the
> length for the internal kernel "small" groups buffer. There is no apparent
> reason to limit to this value.
> 
> This patch removes the 32 groups printing limit.
> 
> The Linux kernel limits the amount of supplementary groups by NGROUPS_MAX,
> which is currently set to 65536. And this is the maximum count of groups we
> may possibly print.
> 
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

The 'min' is older than git history, but at that dawn of time the code
was just sprintf()ing into a large buffer.

I don't *really* see a problem with this, though if someone did have 1000
groups /proc/$$/status would be sort of annoying to read.  So on the one 
hand adding a '...' in /proc/self/status after 32, and adding a /proc/$$/creds
file seems more pleasant, but then you get into the whole adding files to
/proc kerfuffle, so...

Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>

> Cc: stable@vger.kernel.org
> ---
>  fs/proc/array.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> NOTE: I consider this to be a bug which breaks user-space, so I add -stable.
> 
> diff --git a/fs/proc/array.c b/fs/proc/array.c
> index c1c207c..bd31e02 100644
> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -212,7 +212,7 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns,
>  	group_info = cred->group_info;
>  	task_unlock(p);
>  
> -	for (g = 0; g < min(group_info->ngroups, NGROUPS_SMALL); g++)
> +	for (g = 0; g < group_info->ngroups; g++)
>  		seq_printf(m, "%d ",
>  			   from_kgid_munged(user_ns, GROUP_AT(group_info, g)));
>  	put_cred(cred);
> -- 
> 1.7.7.6
> 

  reply	other threads:[~2012-11-09 15:29 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-09 13:31 [PATCH] proc: pid/status: show all supplementary groups Artem Bityutskiy
2012-11-09 15:28 ` Serge Hallyn [this message]
2012-11-09 18:30 ` Kees Cook

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=20121109152854.GA6778@sergelap \
    --to=serge.hallyn@canonical.com \
    --cc=akpm@linux-foundation.org \
    --cc=dedekind1@gmail.com \
    --cc=keescook@chromium.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.