All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Mihai Donțu" <mdontu@bitdefender.com>
To: xen-devel@lists.xen.org
Subject: Re: [PATCH] xmalloc: add support for checking the pool integrity
Date: Thu, 4 Dec 2014 19:10:09 +0200	[thread overview]
Message-ID: <20141204191009.0434bc13@bitdefender.com> (raw)
In-Reply-To: <1417712500-28009-1-git-send-email-mdontu@bitdefender.com>

On Thursday 04 December 2014 19:01:40 Mihai Donțu wrote:
> Implemented xmem_pool_check(), xmem_pool_check_locked() and
> xmem_pool_check_unlocked() to verity the integrity of the TLSF matrix.
> 
> Signed-off-by: Mihai Donțu <mdontu@bitdefender.com>
> ---
>  xen/common/xmalloc_tlsf.c | 119 +++++++++++++++++++++++++++++++++++++++++++++-
>  xen/include/xen/xmalloc.h |   7 +++
>  2 files changed, 124 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
> index a5769c9..009ba60 100644
> --- a/xen/common/xmalloc_tlsf.c
> +++ b/xen/common/xmalloc_tlsf.c
> @@ -120,9 +120,120 @@ struct xmem_pool {
>      char name[MAX_POOL_NAME_LEN];
>  };
>  
> +static struct xmem_pool *xenpool;
> +
> +static inline void MAPPING_INSERT(unsigned long r, int *fl, int *sl);
> +
>  /*
>   * Helping functions
>   */
> +#ifndef NDEBUG
> +static int xmem_pool_check_size(const struct bhdr *b, int fl, int sl)
> +{
> +    while ( b )
> +    {
> +        int __fl;
> +        int __sl;
> +
> +        MAPPING_INSERT(b->size, &__fl, &__sl);
> +        if ( __fl != fl || __sl != sl )
> +        {
> +            printk(XENLOG_ERR "xmem_pool: for block %p size = %u, { fl = %d, sl = %d } should be { fl = %d, sl = %d }\n", b, b->size, fl, sl, __fl, __sl);
> +            return 0;
> +        }
> +        b = b->ptr.free_ptr.next;
> +    }
> +    return 1;
> +}
> +
> +/*
> + * This function must be called from a context where pool->lock is
> + * already acquired
> + */
> +#define xmem_pool_check_unlocked(__pool) __xmem_pool_check_unlocked(__FILE__, __LINE__, __pool)
> +static int __xmem_pool_check_unlocked(const char *file, int line, const struct xmem_pool *pool)
> +{
> +    int i;
> +    int woops = 0;
> +    static int once = 1;
> +
> +    for ( i = 0; i < REAL_FLI; i++ )
> +    {
> +        int fl = ( pool->fl_bitmap & (1 << i) ) ? i : -1;
> +
> +        if ( fl >= 0 )
> +        {
> +            int j;
> +            int bitmap_empty = 1;
> +            int matrix_empty = 1;
> +
> +            for ( j = 0; j < MAX_SLI; j++ )
> +            {
> +                int sl = ( pool->sl_bitmap[fl] & (1 << j) ) ? j : -1;
> +
> +                if ( sl < 0 )
> +                    continue;
> +
> +                if ( once && !pool->matrix[fl][sl] )
> +                {
> +                    /* The bitmap is corrupted */
> +                    printk(XENLOG_ERR "xmem_pool:%s:%d the TLSF bitmap is corrupted\n", file, line);
> +                    __warn((char *)file, line);
> +                    once = 0;
> +                    woops = 1;
> +                }
> +                else if ( once && !xmem_pool_check_size(pool->matrix[fl][sl], fl, sl))
> +                {
> +                    printk(XENLOG_ERR "xmem_pool:%s:%d the TLSF chunk matrix is corrupted\n", file, line);
> +                    __warn((char *)file, line);
> +                    once = 0;
> +                    woops = 1;
> +                }
> +                if ( pool->matrix[fl][sl] )
> +                    matrix_empty = 0;
> +                bitmap_empty = 0;
> +            }
> +            if ( once && bitmap_empty )
> +            {
> +                /* The bitmap is corrupted */
> +                printk(XENLOG_ERR "xmem_pool:%s:%d the TLSF bitmap is corrupted (non-empty FL with empty SL)\n", file, line);
> +                __warn((char *)file, line);
> +                once = 0;
> +                woops = 1;
> +            }
> +            if ( once && matrix_empty )
> +            {
> +                /* The bitmap is corrupted */
> +                printk(XENLOG_ERR "xmem_pool:%s:%d the TLSF bitmap is corrupted (empty matrix)\n", file, line);
> +                __warn((char *)file, line);
> +                once = 0;
> +                woops = 1;
> +            }
> +        }
> +    }
> +
> +    return woops;
> +}
> +
> +#define xmem_pool_check_locked(__pool) __xmem_pool_check_locked(__FILE__, __LINE__, __pool)
> +static int __xmem_pool_check_locked(const char *file, int line, struct xmem_pool *pool)
> +{
> +    int err;
> +
> +    spin_lock(&pool->lock);
> +    err = __xmem_pool_check_unlocked(file, line, pool);
> +    spin_unlock(&pool->lock);
> +    return err;
> +}
> +
> +int __xmem_pool_check(const char *file, int line)
> +{
> +    return __xmem_pool_check_locked(file, line, xenpool);
> +}
> +#else
> +#define xmem_pool_check_locked(__pool) do { if ( 0 && (__pool) ); } while (0)
> +#define xmem_pool_check_unlocked(__pool) do { if ( 0 && (__pool) ); } while (0)
> +#endif
>  
>  /**
>   * Returns indexes (fl, sl) of the list used to serve request of size r
> @@ -381,6 +492,8 @@ void *xmem_pool_alloc(unsigned long size, struct xmem_pool *pool)
>      int fl, sl;
>      unsigned long tmp_size;
>  
> +    xmem_pool_check_locked(pool);
> +
>      if ( pool->init_region == NULL )
>      {
>          if ( (region = pool->get_mem(pool->init_size)) == NULL )
> @@ -442,11 +555,13 @@ void *xmem_pool_alloc(unsigned long size, struct xmem_pool *pool)
>  
>      pool->used_size += (b->size & BLOCK_SIZE_MASK) + BHDR_OVERHEAD;
>  
> +    xmem_pool_check_unlocked(pool);
>      spin_unlock(&pool->lock);
>      return (void *)b->ptr.buffer;
>  
>      /* Failed alloc */
>   out_locked:
> +    xmem_pool_check_unlocked(pool);
>      spin_unlock(&pool->lock);
>  
>   out:
> @@ -464,6 +579,7 @@ void xmem_pool_free(void *ptr, struct xmem_pool *pool)
>      b = (struct bhdr *)((char *) ptr - BHDR_OVERHEAD);
>  
>      spin_lock(&pool->lock);
> +    xmem_pool_check_unlocked(pool);
>      b->size |= FREE_BLOCK;
>      pool->used_size -= (b->size & BLOCK_SIZE_MASK) + BHDR_OVERHEAD;
>      b->ptr.free_ptr = (struct free_ptr) { NULL, NULL};
> @@ -500,6 +616,7 @@ void xmem_pool_free(void *ptr, struct xmem_pool *pool)
>      tmp_b->size |= PREV_FREE;
>      tmp_b->prev_hdr = b;
>   out:
> +    xmem_pool_check_unlocked(pool);
>      spin_unlock(&pool->lock);
>  }
>  
> @@ -512,8 +629,6 @@ int xmem_pool_maxalloc(struct xmem_pool *pool)
>   * Glue for xmalloc().
>   */
>  
> -static struct xmem_pool *xenpool;
> -
>  static void *xmalloc_pool_get(unsigned long size)
>  {
>      ASSERT(size == PAGE_SIZE);
> diff --git a/xen/include/xen/xmalloc.h b/xen/include/xen/xmalloc.h
> index 24a99ac..626ead0 100644
> --- a/xen/include/xen/xmalloc.h
> +++ b/xen/include/xen/xmalloc.h
> @@ -123,4 +123,11 @@ unsigned long xmem_pool_get_used_size(struct xmem_pool *pool);
>   */
>  unsigned long xmem_pool_get_total_size(struct xmem_pool *pool);
>  
> +#ifndef NDEBUG
> +#define xmem_pool_check() __xmem_pool_check(__FILE__, __LINE__)
> +int __xmem_pool_check(const char *file, int line);
> +#else
> +#define xmem_pool_check() do { if ( 0 ); } while (0)
> +#endif
> +
>  #endif /* __XMALLOC_H__ */

This change will generate a message like this one:

[2014-12-04 15:41:23] (XEN) [ 1374.507125] xmem_pool: for block ffff8304004fb9b0 size = 0, { fl = 3, sl = 9 } should be { fl = 0, sl = 0 }
[2014-12-04 15:41:23] (XEN) [ 1374.507127] xmem_pool:xmalloc_tlsf.c:582 the TLSF chunk matrix is corrupted
[2014-12-04 15:41:23] (XEN) [ 1374.507128] Xen WARN at xmalloc_tlsf.c:582
[2014-12-04 15:41:23] (XEN) [ 1374.507131] ----[ Xen-4.4.1  x86_64  debug=y  Not tainted ]----
[2014-12-04 15:41:23] (XEN) [ 1374.507132] CPU:    3
[2014-12-04 15:41:23] (XEN) [ 1374.507133] RIP:    e008:[<ffff82d0801428b0>] __warn+0x1a/0x1e
[2014-12-04 15:41:23] (XEN) [ 1374.507136] RFLAGS: 0000000000010282   CONTEXT: hypervisor
[2014-12-04 15:41:23] (XEN) [ 1374.507138] rax: 0000000000000000   rbx: 0000000000000009   rcx: 0000000000000000
[2014-12-04 15:41:23] (XEN) [ 1374.507139] rdx: ffff830414388000   rsi: 000000000000000a   rdi: ffff82d0802926dc
[2014-12-04 15:41:23] (XEN) [ 1374.507141] rbp: ffff83041438fd38   rsp: ffff83041438fd38   r8:  ffff8304143b0000
[2014-12-04 15:41:23] (XEN) [ 1374.507142] r9:  0000000000000006   r10: 000000000007bcf8   r11: 0000000000000006
[2014-12-04 15:41:23] (XEN) [ 1374.507143] r12: 0000000000000003   r13: 0000000000000001   r14: 0000000000000003
[2014-12-04 15:41:23] (XEN) [ 1374.507145] r15: ffff8304143ca300   cr0: 000000008005003b   cr4: 00000000001526f0
[2014-12-04 15:41:23] (XEN) [ 1374.507146] cr3: 0000000403af6000   cr2: ffffffffff600400
[2014-12-04 15:41:23] (XEN) [ 1374.507147] ds: 002b   es: 002b   fs: 0000   gs: 0000   ss: e010   cs: e008
[2014-12-04 15:41:23] (XEN) [ 1374.507149] Xen stack trace from rsp=ffff83041438fd38:
[2014-12-04 15:41:23] (XEN) [ 1374.507150]    ffff83041438fda8 ffff82d0801324d7 0000000000000000 0000013ff87075a2
[2014-12-04 15:41:23] (XEN) [ 1374.507152]    000000ff96ef1b38 000002468016e35d ffff82d08025a8b9 ffff8304143ca000
[2014-12-04 15:41:23] (XEN) [ 1374.507155]    0000000000000001 ffff83040fc67010 ffff8304143ca000 ffff8304143cb868
[2014-12-04 15:41:23] (XEN) [ 1374.507157]    ffff830402338000 ffff830410dc7dec ffff83041438fdd8 ffff82d080132dc5
[2014-12-04 15:41:23] (XEN) [ 1374.507159]    ffff8300dbfb0030 ffff8300d4bfa000 ffff83040fc67010 ffff830402338da8
[2014-12-04 15:41:23] (XEN) [ 1374.507161]    ffff83041438fe18 ffff82d08013358d ffff8300dbfb0000 ffff8300d4bfa000
[2014-12-04 15:41:23] (XEN) [ 1374.507164]    0000000000000000 ffff830402338da8 ffff830402338000 ffff830410dc7dec
[2014-12-04 15:41:23] (XEN) [ 1374.507166]    ffff83041438fe38 ffff82d0801a2da2 ffff83041438fe48 ffff8300d4bfa000
[2014-12-04 15:41:23] (XEN) [ 1374.507168]    ffff83041438fe48 ffff82d0801675bc ffff83041438fe68 ffff82d080161015
[2014-12-04 15:41:23] (XEN) [ 1374.507171]    ffff8300d4bfa000 ffff8300d4bfa000 ffff83041438fe98 ffff82d080105094
[2014-12-04 15:41:23] (XEN) [ 1374.507173]    ffff8304143962c0 0000000000000000 0000000000000000 ffff830414388000
[2014-12-04 15:41:23] (XEN) [ 1374.507175]    ffff83041438fec8 ffff82d0801337d0 ffff830414363dec ffff82d080308180
[2014-12-04 15:41:23] (XEN) [ 1374.507177]    ffff82d080308000 ffffffffffffffff ffff83041438fef8 ffff82d08012aa36
[2014-12-04 15:41:23] (XEN) [ 1374.507180]    ffff8300d47fe000 ffff8300dbfb0000 0000000000000001 ffff830410dc7000
[2014-12-04 15:41:23] (XEN) [ 1374.507182]    ffff83041438ff08 ffff82d08012aa8f ffff83041438fda0 ffff82d08022bd91
[2014-12-04 15:41:23] (XEN) [ 1374.507184]    ffff880024c43fd8 ffffffff81ab1b38 0000000000000000 0000000000000000
[2014-12-04 15:41:23] (XEN) [ 1374.507186]    ffff880024c43ec0 0000000000000002 0000000000000246 0000000000000000
[2014-12-04 15:41:23] (XEN) [ 1374.507188]    0000000000000000 0000000000000000 0000000000000000 ffffffff810013aa
[2014-12-04 15:41:23] (XEN) [ 1374.507190]    0000000000000000 00000000deadbeef 00000000deadbeef 0002010000000000
[2014-12-04 15:41:23] (XEN) [ 1374.507192]    ffffffff810013aa 000000000000e033 0000000000000246 ffff880024c43ea8
[2014-12-04 15:41:23] (XEN) [ 1374.507194] Xen call trace:
[2014-12-04 15:41:23] (XEN) [ 1374.507196]    [<ffff82d0801428b0>] __warn+0x1a/0x1e
[2014-12-04 15:41:23] (XEN) [ 1374.507198]    [<ffff82d0801324d7>] __xmem_pool_check_unlocked+0x143/0x24f
[2014-12-04 15:41:23] (XEN) [ 1374.507200]    [<ffff82d080132dc5>] xmem_pool_free+0x3f/0x2d9
[2014-12-04 15:41:23] (XEN) [ 1374.507201]    [<ffff82d08013358d>] xfree+0x1d5/0x222
[2014-12-04 15:41:23] (XEN) [ 1374.507203]    [<ffff82d0801a2da2>] xstate_free_save_area+0x18/0x2a
[2014-12-04 15:41:23] (XEN) [ 1374.507205]    [<ffff82d0801675bc>] vcpu_destroy_fpu+0x13/0x23
[2014-12-04 15:41:23] (XEN) [ 1374.507207]    [<ffff82d080161015>] vcpu_destroy+0x26/0x50
[2014-12-04 15:41:23] (XEN) [ 1374.507209]    [<ffff82d080105094>] complete_domain_destroy+0x49/0x16a
[2014-12-04 15:41:23] (XEN) [ 1374.507211]    [<ffff82d0801337d0>] rcu_process_callbacks+0x145/0x1a6
[2014-12-04 15:41:23] (XEN) [ 1374.507213]    [<ffff82d08012aa36>] __do_softirq+0x81/0x8c
[2014-12-04 15:41:23] (XEN) [ 1374.507215]    [<ffff82d08012aa8f>] do_softirq+0x13/0x15
[2014-12-04 15:41:23] (XEN) [ 1374.507253]    [<ffff82d08022bd91>] process_softirqs+0x21/0x30

every time the xmem pool gets corrupted. This is only a hint, though. A
developer will have to place xmem_pool_check() in various places to
determine the source of the corruption. I used it to track down a use
after free bug (details will follow later).

Thanks,

-- 
Mihai DONȚU

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

  reply	other threads:[~2014-12-04 17:10 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-04 17:01 [PATCH] xmalloc: add support for checking the pool integrity Mihai Donțu
2014-12-04 17:10 ` Mihai Donțu [this message]
2014-12-05 12:09 ` Jan Beulich
2014-12-07 23:58   ` Mihai Donțu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141204191009.0434bc13@bitdefender.com \
    --to=mdontu@bitdefender.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.