Hi,
I am testing my kmemcheck patches, and it has come up with a couple of

uses of uninitialized memory in lib/idr.c. These are (the line numbers

may differ slightly):
line 135 (sub_alloc): bm = ~p->bitmap;

p->bitmap is uninitialized
line 171 (sub_alloc): if (!p->ary[m]) {

p->ary is uninitialized
line 249 (idr_get_new_above_int): pa[0]->count++;

pa[0]->count is uninitialized
I cannot guarantee that these are truly errors, but I would be

grateful if you could help me confirm/deny the validity of the

reports. Personally, I can get rid of the errors using this patch:
diff --git a/lib/idr.c b/lib/idr.c

index afbb0b1..dd28ee5 100644

--- a/lib/idr.c

+++ b/lib/idr.c

@@ -39,12 +39,16 @@ static struct idr_layer *alloc_layer(struct idr *idp)

 {

        struct idr_layer *p;

        unsigned long flags;

+       int i;
        spin_lock_irqsave(&idp->lock, flags);

        if ((p = idp->id_free)) {

                idp->id_free = p->ary[0];

                idp->id_free_cnt--;

-               p->ary[0] = NULL;

+               p->bitmap = 0;

+               for(i = 0; i < ARRAY_SIZE(p->ary); ++i)

+                       p->ary[i] = NULL;

+               p->count = 0;

        }

        spin_unlock_irqrestore(&idp->lock, flags);

        return(p);
Thanks a lot.
Vegard

--

Hi Vegard,
                        Pekka

--

That would make sense. However...
idr_layer_cache is only used for allocations from idr_pre_get().

idr_pre_get() is only called from ida_pre_get().
If this analysis is correct, can this mean that the user has failed to

call ida_pre_get() before idr_get_new() was called?
Though in this case, idr_pre_get() actually *is* called first. Hmm...

I think there's a pretty big chance that kmemcheck is at fault :-(
Vegard

--

Hi Vegard,
Depends on how you track object initialization. An object returned by

kmem_cache_alloc() is always initialized if the cache it comes from has a

constructor.
			Pekka

--

I think there's a pretty big chance I'm wrong (or misunderstanding

something) here, so I'll just ask:

setup_object() from mm/slub.c is what calls the ctor. Shouldn't this

be called from slab_alloc() as well? (I'm marking the data

"uninitialized" there before returning the object.) Otherwise you

might get back an object that is initialized with the previous owner's

data. Or is this intentional?
Thanks.
Vegard

--

Hi Vegard,
It's intentional. The caller of kmem_cache_free() is expected to put

the object in such a state that it can be recycled immediately when

kmem_cache_alloc() for that cache is called. You can find the design

rationale for that in Bonwick's original paper on slab:

http://citeseer.ist.psu.edu/bonwick94slab.html
                        Pekka

--

Ow. I guess this is the end of the thread, then :-) My fault. Thanks!
Vegard

--