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

[Vegard Nossum]

2009/8/25 Pekka Enberg <penberg@xxxxxxxxxxxxxx>:
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/