From: ebiederm@xmission.com (Eric W. Biederman)
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Andy Lutomirski <luto@amacapital.net>,
Wang YanQing <udknight@gmail.com>,
linux-kernel@vger.kernel.org, linux-nfs@vger.kernel.org,
kernel-janitors@vger.kernel.org
Subject: Re: [patch] groups: integer underflow in groups_alloc()
Date: Mon, 23 Feb 2015 12:46:37 -0600 [thread overview]
Message-ID: <87fv9wzcs2.fsf@x220.int.ebiederm.org> (raw)
In-Reply-To: <20150223180326.GC5116@mwanda> (Dan Carpenter's message of "Mon, 23 Feb 2015 21:03:27 +0300")
Dan Carpenter <dan.carpenter@oracle.com> writes:
> On Mon, Feb 23, 2015 at 11:10:02AM -0600, Eric W. Biederman wrote:
>> Dan Carpenter <dan.carpenter@oracle.com> writes:
>>
>> > This is called from rsc_parse() with a use controlled value. Say for
>> > example that "gidsetsize" is negative, then we could end up allocating
>> > less than sizeof(struct group_info) leading to memory corruption.
>>
>> Right now it is the responsibility of the caller of groups_alloc to make
>> certain that gidsetsize is a valid value, and the callers of
>> groups_alloc who know what they are doing already validate this value.
>>
>> Either the pattern of caller validates the messages needs to continue,
>> or groups_alloc needs to be changed and all of the callers need to be
>> updated.
>>
>> Changing groups_alloc for one particular caller is just going to cause
>> maintenance problems.
>>
>
> This only affects NFS so let's hear from them if this limit is correct
> and decide from there.
The bug may be nfs specific bug changing groups_alloc does not only
affect nfs.
NGROUPS_MAX is the maxmimum number of groups the linux kernel supports
so NGROUPS_MAX may be high but it is certainly not wrong.
Your patch takes the wrong approach, creates code that is an
inconsistent mess and is thus wrong. As setgroups is code that is
called every day I don't think only paying attention to NFS when talking
how to change this is in any way appropriate, unless you propose an NFS
specific fix (which you clearly did not).
Eric
next prev parent reply other threads:[~2015-02-23 18:50 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-23 15:44 [patch] groups: integer underflow in groups_alloc() Dan Carpenter
2015-02-23 17:10 ` Eric W. Biederman
2015-02-23 18:03 ` Dan Carpenter
2015-02-23 18:46 ` Eric W. Biederman [this message]
2015-02-23 21:16 ` J. Bruce Fields
2015-02-24 15:34 ` [patch v2] sunrpc: integer underflow in rsc_parse() Dan Carpenter
2015-02-25 3:54 ` Simo Sorce
2015-02-26 20:40 ` J. Bruce Fields
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=87fv9wzcs2.fsf@x220.int.ebiederm.org \
--to=ebiederm@xmission.com \
--cc=akpm@linux-foundation.org \
--cc=dan.carpenter@oracle.com \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=udknight@gmail.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