Re: WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (f6f6e1a4), by kmemleak's scan_block()

[Vegard Nossum <vegard.nossum@gmail.com>]

2009/8/25 Pekka Enberg <penberg@cs.helsinki.fi>:
> I thik this should be fine for -tip.
>
>                        Pekka
>
> diff --git a/arch/x86/mm/kmemcheck/kmemcheck.c b/arch/x86/mm/kmemcheck/kmemcheck.c
> index 2c55ed0..528bf95 100644
> --- a/arch/x86/mm/kmemcheck/kmemcheck.c
> +++ b/arch/x86/mm/kmemcheck/kmemcheck.c
> @@ -331,6 +331,20 @@ static void kmemcheck_read_strict(struct pt_regs *regs,
>        kmemcheck_shadow_set(shadow, size);
>  }
>
> +bool kmemcheck_is_obj_initialized(unsigned long addr, size_t size)
> +{
> +       enum kmemcheck_shadow status;
> +       void *shadow;
> +
> +       shadow = kmemcheck_shadow_lookup(addr);
> +       if (!shadow)
> +               return true;
> +
> +       status = kmemcheck_shadow_test(shadow, size);
> +
> +       return status == KMEMCHECK_SHADOW_INITIALIZED;
> +}
> +
>  /* Access may cross page boundary */
>  static void kmemcheck_read(struct pt_regs *regs,
>        unsigned long addr, unsigned int size)
> diff --git a/include/linux/kmemcheck.h b/include/linux/kmemcheck.h
> index 47b39b7..dc2fd54 100644
> --- a/include/linux/kmemcheck.h
> +++ b/include/linux/kmemcheck.h
> @@ -34,6 +34,8 @@ void kmemcheck_mark_initialized_pages(struct page *p, unsigned int n);
>  int kmemcheck_show_addr(unsigned long address);
>  int kmemcheck_hide_addr(unsigned long address);
>
> +bool kmemcheck_is_obj_initialized(unsigned long addr, size_t size);
> +
>  #else
>  #define kmemcheck_enabled 0
>
> @@ -99,6 +101,11 @@ static inline void kmemcheck_mark_initialized_pages(struct page *p,
>  {
>  }
>
> +static inline bool kmemcheck_is_obj_initialized(unsigned long addr, size_t size)
> +{
> +       return true;
> +}
> +
>  #endif /* CONFIG_KMEMCHECK */
>
>  /*
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index 6debe0d..73947e4 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -97,6 +97,7 @@
>  #include <asm/processor.h>
>  #include <asm/atomic.h>
>
> +#include <linux/kmemcheck.h>
>  #include <linux/kmemleak.h>
>
>  /*
> @@ -951,6 +952,8 @@ static void scan_object(struct kmemleak_object *object)
>        if (!(object->flags & OBJECT_ALLOCATED))
>                /* already freed object */
>                goto out;
> +       if (!kmemcheck_is_obj_initialized(object->pointer, object->size))
> +               goto out;
>        if (hlist_empty(&object->area_list)) {
>                void *start = (void *)object->pointer;
>                void *end = (void *)(object->pointer + object->size);
>
>
>


I don't know so much about the kmemleak internals, but this I can say
about the kmemcheck part: According to your definition, an object is
initialized if all the bytes of an object are initialized.

Is it possible that because of this, if we have a partially
uninitialized object, kmemleak will not record the pointers found in
that object? If so, it might skip valid pointers, and deem an object
unreferenced. Which could make kmemleak give false-positives.

I think it would be better to ask kmemcheck on a per-pointer basis
(i.e. for each pointer-sized word in the object), whether it is
initialized or not.


Vegard