use after free bug in b98043a2f8e7bb5b1918e2e02778f822f9dd4d3a

use after free bug in b98043a2f8e7bb5b1918e2e02778f822f9dd4d3a

[Jes Sorensen]

Hi,

I was looking at ExamineBitmap() and noticed that your patch below,
seems to introduce a use after free bug.

commit b98043a2f8e7bb5b1918e2e02778f822f9dd4d3a
Author: Guoqing Jiang <gqjiang@suse.com>
Date:   Wed Jun 10 13:42:07 2015 +0800

    Show all bitmaps while examining bitmap
    
    This adds capability of exmining bitmaps corresponding to all
    nodes/slots on the device.
    
    Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
    Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
    Signed-off-by: NeilBrown <neilb@suse.de>

Line 284 does a close(fd), but further down the code your patch
introduces:

+       } else {
+               printf("   Cluster nodes : %d\n", sb->nodes);
+               printf("    Cluster name : %64s\n", sb->cluster_name);
+               for (i = 0; i < (int)sb->nodes; i++) {
+                       if (i) {
+                               free(info);
+                               info = bitmap_fd_read(fd, brief);
+                               sb = &info->sb;
+                       }

It's not totally obvious to me here what the intention is for clustered
md here. Are you intending to use the same fd as was used at the top of
ExamineBitmap() or is it meant to open a new fd based on each clustered
entry?

In either case, the code as it is right now is certainly not going to
work :(

Cheers,
Jes

Re: use after free bug in b98043a2f8e7bb5b1918e2e02778f822f9dd4d3a

[Guoqing Jiang]

On 03/05/2016 05:19 AM, Jes Sorensen wrote:
> Hi,
>
> I was looking at ExamineBitmap() and noticed that your patch below,
> seems to introduce a use after free bug.
>
> commit b98043a2f8e7bb5b1918e2e02778f822f9dd4d3a
> Author: Guoqing Jiang <gqjiang@suse.com>
> Date:   Wed Jun 10 13:42:07 2015 +0800
>
>      Show all bitmaps while examining bitmap
>      
>      This adds capability of exmining bitmaps corresponding to all
>      nodes/slots on the device.
>      
>      Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
>      Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
>      Signed-off-by: NeilBrown <neilb@suse.de>
>
> Line 284 does a close(fd), but further down the code your patch
> introduces:
>
> +       } else {
> +               printf("   Cluster nodes : %d\n", sb->nodes);
> +               printf("    Cluster name : %64s\n", sb->cluster_name);
> +               for (i = 0; i < (int)sb->nodes; i++) {
> +                       if (i) {
> +                               free(info);
> +                               info = bitmap_fd_read(fd, brief);
> +                               sb = &info->sb;
> +                       }
>
> It's not totally obvious to me here what the intention is for clustered
> md here. Are you intending to use the same fd as was used at the top of
> ExamineBitmap() or is it meant to open a new fd based on each clustered
> entry?
>
> In either case, the code as it is right now is certainly not going to
> work :(

Yes, it is wrong, thanks for the catch, I will handle it too.

Best Regards,
Guoqing