All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Paris <eparis@redhat.com>
To: Dave Hansen <dave@linux.vnet.ibm.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org
Subject: Re: [PATCH] flex_array: Change behaviour on zero size allocations
Date: Tue, 01 Feb 2011 10:20:22 -0500	[thread overview]
Message-ID: <1296573623.3145.6.camel@localhost.localdomain> (raw)
In-Reply-To: <1296572145.27022.2837.camel@nimitz>

On Tue, 2011-02-01 at 06:55 -0800, Dave Hansen wrote:
> On Tue, 2011-02-01 at 12:03 +0100, Steffen Klassert wrote:
> > rc = flex_array_prealloc(p->type_val_to_struct_array, 0,
> >                          p->p_types.nprim - 1, GFP_KERNEL | __GFP_ZERO);
> > if (rc)
> >         goto out;
> > 
> > If p->p_types.nprim is zero, we allocare with total_nr_elements equal
> > to zerro and then we try to prealloc with p->p_types.nprim - 1.
> > flex_array_prealloc interprets this as an unsigned int and fails,
> > because this is bigger than total_nr_elements, which is correct I
> > think.
> > 
> > Thoughts?
> 
> The most we ever hold in a flex_array is ~2 million entries.  So we have
> plenty of room to use a normal int if you want.
> 
> On the other hand, there's only one user of flex_array_prealloc(), and
> making the "end" argument inclusive doesn't seem to be what that user
> wants.  We might want to either make flex_array_prealloc() take start
> and length, or instead make "end" be exclusive of the "end" index.
>
> I thought that flex_array_prealloc would say, effectively: "all put()'s
> would work up until 'end'".  But, looking at it now, that's probably not
> how people will use it.  

I'm fine with any solution.  It's obviously broken for SELinux to be
passing -1 even if the library supported it.  I guess I don't really
have strong feelings on how to fix it.

1) make end exclusive
2) change 'end' to 'len'
3) just make selinux not prealloc() when the #elements == 0

All seem perfectly reasonable to me, but I'd probably do them in that
order.

-Eric


  reply	other threads:[~2011-02-01 15:20 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-31  8:52 [PATCH] flex_array: Change behaviour on zero size allocations Steffen Klassert
2011-01-31 16:31 ` Dave Hansen
2011-02-01 10:24   ` Steffen Klassert
2011-02-01 11:03     ` Steffen Klassert
2011-02-01 14:55       ` Dave Hansen
2011-02-01 15:20         ` Eric Paris [this message]
2011-02-02  7:55           ` Steffen Klassert

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=1296573623.3145.6.camel@localhost.localdomain \
    --to=eparis@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=dave@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=steffen.klassert@secunet.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 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.