On Fri, 1 Jun 2007, [EMAIL PROTECTED] wrote:
> On Fri, 01 Jun 2007 16:00:30 PDT, Linus Torvalds said:
>
> > #define BADPTR ((void *)16)
>
> > I bet you'd find *more* problems that way than by returning NULL, and
> > you'd also avoid the whole problem with "if (!ptr) return -ENOMEM".
>
> Hm
On Fri, 01 Jun 2007 16:00:30 PDT, Linus Torvalds said:
> #define BADPTR ((void *)16)
> I bet you'd find *more* problems that way than by returning NULL, and
> you'd also avoid the whole problem with "if (!ptr) return -ENOMEM".
Hmm.. this looks like a good contender for "first usage of #if
On Fri, 1 Jun 2007, Christoph Lameter wrote:
>
> On Fri, 1 Jun 2007, Andrew Morton wrote:
>
> > I think it'd be better if we kept the WARN_ON_ONCE(size == 0) in there,
>
> The trouble with the WARN_ON is that it triggers even for code that is
> okay like noted by Jeremy.
Yes. Sometimes it's j
Andrew Morton wrote:
> In some cases, sure, and we should remove the warning at some stage.
>
> But it has exposed a couple of bugs and a few weird things.
>
We just need to scatter better bug powder into the corners of the
allocations.
J
-
To unsubscribe from this list: send the line "unsubsc
On Fri, 01 Jun 2007 17:43:46 -0700 Jeremy Fitzhardinge <[EMAIL PROTECTED]>
wrote:
> Andrew Morton wrote:
> > As I said, it's specific to the kmalloc(0) problem, and we're fixing that
> > by other means anyway.
> >
>
> I guess I'm not getting much traction in my campaign for equal rights
> for
Linus Torvalds wrote:
> So for *both* of the above reasons, it's actually stupid to return NULL
> for a zero-sized allocation. It would be much better to return another
> pointer that will trap on access. A good candidate might be to return
>
> #define BADPTR ((void *)16)
>
I think this
Andrew Morton wrote:
> As I said, it's specific to the kmalloc(0) problem, and we're fixing that
> by other means anyway.
>
I guess I'm not getting much traction in my campaign for equal rights
for zero-sized allocations. They're perfectly reasonable things to have,
if that's the way the code f
Christoph Lameter wrote:
> Well there are architectural problems. We determine the power of two slab
> at compile time. The object size information is currently not available in
> the binary :=).
>
That only applies to allocations with constant sizes. One presumes
nobody is explicitly doing k
On Fri, 1 Jun 2007, Andrew Morton wrote:
> I think it'd be better if we kept the WARN_ON_ONCE(size == 0) in there,
The trouble with the WARN_ON is that it triggers even for code that is
okay like noted by Jeremy. My initial intend with NULL was to allow the
allocation of a zero sized pointer wi
On Fri, 1 Jun 2007 16:57:20 -0700 (PDT)
Linus Torvalds <[EMAIL PROTECTED]> wrote:
> Andrew, want to take this patch to -mm to see if it triggers anything?
spose so.
I think it'd be better if we kept the WARN_ON_ONCE(size == 0) in there,
because it is exposing some coding warts. But we should tu
On Fri, 1 Jun 2007, Linus Torvalds wrote:
> > A too large alloc is >32MB or MAX_ORDER << PAGE_SIZE. A BUG_ON in
> > kmalloc_slab() will trigger.
>
> Did we use to BUG_ON()? I think that's wrong. There are ways for users to
> potentially ask the kernel to do big allocations, and the correct resp
On Fri, 1 Jun 2007, Christoph Lameter wrote:
>
> A too large alloc is >32MB or MAX_ORDER << PAGE_SIZE. A BUG_ON in
> kmalloc_slab() will trigger.
Did we use to BUG_ON()? I think that's wrong. There are ways for users to
potentially ask the kernel to do big allocations, and the correct respons
On Fri, 1 Jun 2007, Linus Torvalds wrote:
> I'm not seeing the path on the freeing side that silently accepts BADPTR.
>
> It is not ok to derefence BADPTR, but it's obviously ok to _free_ it.
Ok.
> Also, if I read the patch correctly, you _also_ return BADPTR for slabs
> that are too large. No
On Fri, 1 Jun 2007, Christoph Lameter wrote:
>
> Something like this? (Not tested yet just for review):
I'm not seeing the path on the freeing side that silently accepts BADPTR.
It is not ok to derefence BADPTR, but it's obviously ok to _free_ it.
Also, if I read the patch correctly, you _als
On Fri, 1 Jun 2007, Linus Torvalds wrote:
> No, I don't think you can do it this way.
Ultimately not. But its worth to see if this works.
> At a minimum, you'd need to test that the result is word-aligned.
> Preferably 8-byte aligned. We literally have stuff that knows about these
> things and
On Fri, 1 Jun 2007, Christoph Lameter wrote:
>
> Hmmm... We are going rapidly here. This is a patch that I am testing right
> now. It right adjust the object and the patch is manageable:
No, I don't think you can do it this way.
At a minimum, you'd need to test that the result is word-aligned
On Fri, 1 Jun 2007, Linus Torvalds wrote:
> So for *both* of the above reasons, it's actually stupid to return NULL
> for a zero-sized allocation. It would be much better to return another
> pointer that will trap on access. A good candidate might be to return
>
> #define BADPTR ((void *)
On Fri, 1 Jun 2007, Andrew Morton wrote:
>
> We could store the size of the allocation in the allocated object? Just
> add four bytes to the user's request, then pick the appropriate cache based
> on that, then put the user's `size' at the tail of the resulting allocation?
It should be easy en
On Fri, 1 Jun 2007, Christoph Lameter wrote:
> Hmmm... We are going rapidly here. This is a patch that I am testing right
> now. It right adjust the object and the patch is manageable:
Does not boot sigh.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a
> So a kmalloc(62) would get upped to 66, so we allocate from size-128
> and put the number 62 at bytes 124-127 and we poison bytes 62-123?
Hmmm... We are going rapidly here. This is a patch that I am testing right
now. It right adjust the object and the patch is manageable:
SLUB mm-only: Righ
On Fri, 1 Jun 2007 15:41:48 -0700 (PDT)
Christoph Lameter <[EMAIL PROTECTED]> wrote:
> On Fri, 1 Jun 2007, Andrew Morton wrote:
>
> > > I should make SLUB put poisoning values in unused areas of a kmalloced
> > > object?
> >
> > hm, I hadn't thought of it that way actually. I was thinking it w
On Fri, 1 Jun 2007, Christoph Lameter wrote:
>
> Right it could catch a lot of other bugs as well.
I'd actually prefer "malloc(0)" to _not_ return NULL, but some known
(non-NULL) bogus pointer.
Why?
Because it's quite sane to have simple logic like
ptr = malloc(size);
if (!p
On Fri, 1 Jun 2007, Andrew Morton wrote:
> > I should make SLUB put poisoning values in unused areas of a kmalloced
> > object?
>
> hm, I hadn't thought of it that way actually. I was thinking it was
> specific to kmalloc(0) but as you point out, the situation is
> generalisable.
Right it coul
On Fri, 1 Jun 2007 15:20:05 -0700 (PDT)
Christoph Lameter <[EMAIL PROTECTED]> wrote:
> On Fri, 1 Jun 2007, Andrew Morton wrote:
>
> > If slab was smart enough, it would have poisoned those 8 bytes to some
> > known pattern, and then checked that they still had that pattern when the
> > memory got
On Fri, 1 Jun 2007, Andrew Morton wrote:
> If slab was smart enough, it would have poisoned those 8 bytes to some
> known pattern, and then checked that they still had that pattern when the
> memory got freed again.
So this is new feature request?
> But it isn't smart enough, so the bug went und
On Fri, 1 Jun 2007 14:45:27 -0700 (PDT)
Christoph Lameter <[EMAIL PROTECTED]> wrote:
> On Fri, 1 Jun 2007, Andrew Morton wrote:
>
> > Poisoning and redzoning could have caught that.
>
> Redzoning would not have caught it. This was a kmalloc allocation and
> SLAB always gave them 32 bytes to pla
On Fri, 1 Jun 2007, Andrew Morton wrote:
> Poisoning and redzoning could have caught that.
Redzoning would not have caught it. This was a kmalloc allocation and
SLAB always gave them 32 bytes to play with. Only writes more than 32
bytes behind would have been caught.
Poisoning is only applicab
On Fri, 1 Jun 2007 13:47:23 -0700 (PDT)
Christoph Lameter <[EMAIL PROTECTED]> wrote:
> On Fri, 1 Jun 2007, Jeremy Fitzhardinge wrote:
>
> > I think this is a good example of why having to special-case kmalloc(0)
> > is a bad idea. The original code was straightforward and, barring
> > silliness,
Christoph Lameter wrote:
> On Fri, 1 Jun 2007, Jeremy Fitzhardinge wrote:
>
>
>> I think this is a good example of why having to special-case kmalloc(0)
>> is a bad idea. The original code was straightforward and, barring
>> silliness, should be completely correct with npids==0. This new code
Christoph wrote:
> Then you are deferencing an element in the pidarray that you did not
> allocate! This is a bug in cpuset code.
Absolutely - as I described in more detail in a reply Jeremy a
few minutes ago. Thanks for smoking it out.
If, as I suggested in my previous message to which you are
On Fri, 1 Jun 2007, Jeremy Fitzhardinge wrote:
> I think this is a good example of why having to special-case kmalloc(0)
> is a bad idea. The original code was straightforward and, barring
> silliness, should be completely correct with npids==0. This new code
> does nothing other than make thing
On Fri, 1 Jun 2007, Paul Jackson wrote:
> > There are no checks necessary. Your function worked fine so far for
> > the case of zero objects with the pointer returned by kmalloc. If the
> > code is correct then it will not dereference the pointer to the zero
> > sized array. If not then we may
> I think this is a good example of why having to special-case kmalloc(0)
> is a bad idea. The original code was straightforward and, barring
> silliness, should be completely correct with npids==0. This new code
> does nothing other than make things more complex.
Perhaps so. Perhaps not.
Actu
Srinivasa Ds wrote:
> ---
> kernel/cpuset.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> Index: linux-2.6.22-rc3/kernel/cpuset.c
> ===
> --- linux-2.6.22-rc3.orig/kernel/cpuset.c
> +++ linux-2.6.22-rc3/kernel/cpuset.c
> @
> There are no checks necessary. Your function worked fine so far for
> the case of zero objects with the pointer returned by kmalloc. If the
> code is correct then it will not dereference the pointer to the zero
> sized array. If not then we may find a bug and fix it.
I suspect you got lucky.
On Fri, 1 Jun 2007, Paul Jackson wrote:
> > We must have a NULL pointer exactly because it cannot be dereferenced.
>
> Well, both patch versions had NULL pointers - either pidarray or ctr->buf.
>
> But now I can make more sense of what you say -- you -want- the pidarray
> pointer, in particular,
> We must have a NULL pointer exactly because it cannot be dereferenced.
Well, both patch versions had NULL pointers - either pidarray or ctr->buf.
But now I can make more sense of what you say -- you -want- the pidarray
pointer, in particular, to be NULL, so that we don't accidentally use it.
D
On Fri, 1 Jun 2007, Paul Jackson wrote:
> > The allocated array has 0 bytes. Any dereference is an error.
>
> Well, then, that doesn't work either.
>
> We -agree- we can't have this failing. You suggested (if I understood
> correctly, which I doubt) that we set the pointer to NULL, and noted
> t
> The allocated array has 0 bytes. Any dereference is an error.
Well, then, that doesn't work either.
We -agree- we can't have this failing. You suggested (if I understood
correctly, which I doubt) that we set the pointer to NULL, and noted
that derefencing a NULL pointer would fail. I agree suc
On Fri, 1 Jun 2007, Paul Jackson wrote:
> What do you mean, Christoph, by "then any dereferences will fail" ?
> That sounds bad to me -- we don't want pointer dereferences failing
> in the kernel, do we? Wouldn't you have to add some more lines of
> code, a bit further down, where pidarray is use
Christoph wrote:
> H Isnt it easier to set the pidarray to NULL in the case it
> has no objects? Then any deferences will fail. Paul?
>From what I can tell, in 30 seconds of reading, either of these
approaches, Christoph's or Srinivasa's, based on Vatsa's suggestions
-could- be made to wor
On Fri, 1 Jun 2007, Srinivasa Ds wrote:
> I have modified the patch little bit, Thanks Vatsa for input and hence
> resending it. Please let me know your comments on this.
H Isnt it easier to set the pidarray to NULL in the case it
has no objects? Then any deferences will fail. Paul?
--
On Friday 01 June 2007 12:57:53 Srinivasa Ds wrote:
I have modified the patch little bit, Thanks Vatsa for input and hence
resending it. Please let me know your comments on this.
Signed-off-by: Srinivasa DS <[EMAIL PROTECTED]>
---
kernel/cpuset.c | 10 ++
1 file changed, 10 insertion
43 matches
Mail list logo
