| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20100531164121.GV17639@pcarmody-desktop>
Date: Mon, 31 May 2010 19:41:21 +0300
From: Phil Carmody <ext-phil.2.carmody@...ia.com>
To: "Doyu Hiroshi (Nokia-D/Helsinki)" <hiroshi.doyu@...ia.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"catalin.marinas@....com" <catalin.marinas@....com>
Subject: Re: [PATCH 1/2] kmemleak: Fix some false positives with special
scan
One small comment below.
On 14/05/10 09:16 +0200, Doyu Hiroshi (Nokia-D/Helsinki) wrote:
> From: Hiroshi DOYU <Hiroshi.DOYU@...ia.com>
>
> There is the false positive that the pointer is calculated by other
> methods than the usual container_of macro. "kmemleak_ignore" can cover
> a false positive, but it would loose the advantage of kmemleak. This
> patch allows kmemleak to work with such false positives by introducing
> a new special memory block with a calculation formula. The client
> module can register the area with a function, which kmemleak scan and
> calculate the pointer with the function.
>
> The typical use case could be the IOMMU first level pagetable which
> stores the pointer to the second level of page table with
> modification, for example, a physical address with attribution bits.
>
> Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@...ia.com>
> ---
> include/linux/kmemleak.h | 4 ++
> mm/kmemleak.c | 83 ++++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 84 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/kmemleak.h b/include/linux/kmemleak.h
> index 99d9a67..10be9ef 100644
> --- a/include/linux/kmemleak.h
> +++ b/include/linux/kmemleak.h
> @@ -35,6 +35,10 @@ extern void kmemleak_ignore(const void *ptr) __ref;
> extern void kmemleak_scan_area(const void *ptr, size_t size, gfp_t gfp) __ref;
> extern void kmemleak_no_scan(const void *ptr) __ref;
>
> +extern int kmemleak_special_scan(const void *ptr, size_t size,
> + unsigned long (*fn)(unsigned long)) __ref;
> +extern void kmemleak_no_special(const void *ptr) __ref;
> +
> static inline void kmemleak_alloc_recursive(const void *ptr, size_t size,
> int min_count, unsigned long flags,
> gfp_t gfp)
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index 2c0d032..5166987 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -249,6 +249,67 @@ static struct early_log
> early_log[CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE] __initdata;
> static int crt_early_log __initdata;
>
> +/* scan area which requires special conversion */
> +struct special_block {
> + void *start;
> + void *end;
> + unsigned long (*fn)(unsigned long);
> +};
> +#define SPECIAL_MAX 5
> +static struct special_block special_block[SPECIAL_MAX];
> +static DEFINE_SPINLOCK(special_block_lock);
> +
> +int kmemleak_special_scan(const void *ptr, size_t size,
> + unsigned long (*fn)(unsigned long))
> +{
> + struct special_block *p;
> + int i, err = 0;
> +
> + if (!ptr || (size == 0) || !fn)
> + return -EINVAL;
> +
> + spin_lock(&special_block_lock);
> +
> + p = special_block;
> + for (i = 0; i < SPECIAL_MAX; i++, p++) {
> + if (!p->start)
> + break;
> + }
> +
> + if (i == SPECIAL_MAX) {
> + err = -ENOMEM;
> + goto out;
> + }
> + p->start = (void *)ptr;
> + p->end = (void *)ptr + size;
> + p->fn = fn;
> +out:
> + spin_unlock(&special_block_lock);
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(kmemleak_special_scan);
> +
> +void kmemleak_no_special(const void *ptr)
> +{
> + int i;
> +
> + spin_lock(&special_block_lock);
> +
> + for (i = 0; i < SPECIAL_MAX; i++) {
> + struct special_block *p;
> +
> + p = &special_block[i];
> + if (p->start == ptr) {
> + memset(p, 0, sizeof(*p));
> + break;
> + }
> + }
> +
> + spin_unlock(&special_block_lock);
> +}
> +EXPORT_SYMBOL_GPL(kmemleak_no_special);
> +
> static void kmemleak_disable(void);
>
> /*
> @@ -983,8 +1044,9 @@ static int scan_should_stop(void)
> * Scan a memory block (exclusive range) for valid pointers and add those
> * found to the gray list.
> */
> -static void scan_block(void *_start, void *_end,
> - struct kmemleak_object *scanned, int allow_resched)
> +static void __scan_block(void *_start, void *_end,
> + struct kmemleak_object *scanned, int allow_resched,
> + unsigned long (*fn)(unsigned long))
> {
> unsigned long *ptr;
> unsigned long *start = PTR_ALIGN(_start, BYTES_PER_POINTER);
> @@ -1005,7 +1067,7 @@ static void scan_block(void *_start, void *_end,
> BYTES_PER_POINTER))
> continue;
>
> - pointer = *ptr;
> + pointer = fn ? fn(*ptr) : *ptr;
Tests on the real-world scenario where this special scan became
desirable indicate that the following micro-optimisation is useful,
as much of the scanning is over zero-initialised blocks:
- pointer = *ptr;
+ pointer = (fn && *ptr) ? fn(*ptr) : *ptr;
But that's itsy-bitsy.
As this patchset is already making itself useful, I'd like to add my
support for it:
Acked-by: Phil Carmody <ext-phil.2.carmody@...ia.com>
Cheers,
Phil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists