* Btrfs: check for empty bitmap list in setup_cluster_bitmaps
@ 2015-12-15 17:08 Chris Mason
2015-12-15 18:37 ` Josef Bacik
2015-12-16 1:48 ` Manish
0 siblings, 2 replies; 4+ messages in thread
From: Chris Mason @ 2015-12-15 17:08 UTC (permalink / raw)
To: Btrfs mailing list; +Cc: Andrey Ryabinin, Dave Jones
Dave Jones found a warning from kasan in setup_cluster_bitmaps()
==================================================================
BUG: KASAN: stack-out-of-bounds in setup_cluster_bitmap+0xc4/0x5a0 at
addr ffff88039bef6828
Read of size 8 by task nfsd/1009
page:ffffea000e6fbd80 count:0 mapcount:0 mapping: (null)
index:0x0
flags: 0x8000000000000000()
page dumped because: kasan: bad access detected
CPU: 1 PID: 1009 Comm: nfsd Tainted: G W
4.4.0-rc3-backup-debug+ #1
ffff880065647b50 000000006bb712c2 ffff88039bef6640 ffffffffa680a43e
0000004559c00000 ffff88039bef66c8 ffffffffa62638d1 ffffffffa61121c0
ffff8803a5769de8 0000000000000296 ffff8803a5769df0 0000000000046280
Call Trace:
[<ffffffffa680a43e>] dump_stack+0x4b/0x6d
[<ffffffffa62638d1>] kasan_report_error+0x501/0x520
[<ffffffffa61121c0>] ? debug_show_all_locks+0x1e0/0x1e0
[<ffffffffa6263948>] kasan_report+0x58/0x60
[<ffffffffa6814b00>] ? rb_last+0x10/0x40
[<ffffffffa66f8af4>] ? setup_cluster_bitmap+0xc4/0x5a0
[<ffffffffa6262ead>] __asan_load8+0x5d/0x70
[<ffffffffa66f8af4>] setup_cluster_bitmap+0xc4/0x5a0
[<ffffffffa66f675a>] ? setup_cluster_no_bitmap+0x6a/0x400
[<ffffffffa66fcd16>] btrfs_find_space_cluster+0x4b6/0x640
[<ffffffffa66fc860>] ? btrfs_alloc_from_cluster+0x4e0/0x4e0
[<ffffffffa66fc36e>] ? btrfs_return_cluster_to_free_space+0x9e/0xb0
[<ffffffffa702dc37>] ? _raw_spin_unlock+0x27/0x40
[<ffffffffa666a1a1>] find_free_extent+0xba1/0x1520
Andrey noticed this was because we were doing list_first_entry on a list
that might be empty. Rework the tests a bit so we don't do that.
Signed-off-by: Chris Mason <clm@fb.com>
Reprorted-by: Andrey Ryabinin <ryabinin.a.a@gmail.com>
Reported-by: Dave Jones <dsj@fb.com>
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 0948d34..e6fc7d9 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -2972,7 +2972,7 @@ setup_cluster_bitmap(struct btrfs_block_group_cache *block_group,
u64 cont1_bytes, u64 min_bytes)
{
struct btrfs_free_space_ctl *ctl = block_group->free_space_ctl;
- struct btrfs_free_space *entry;
+ struct btrfs_free_space *entry = NULL;
int ret = -ENOSPC;
u64 bitmap_offset = offset_to_bitmap(ctl, offset);
@@ -2983,8 +2983,10 @@ setup_cluster_bitmap(struct btrfs_block_group_cache *block_group,
* The bitmap that covers offset won't be in the list unless offset
* is just its start offset.
*/
- entry = list_first_entry(bitmaps, struct btrfs_free_space, list);
- if (entry->offset != bitmap_offset) {
+ if (!list_empty(bitmaps))
+ entry = list_first_entry(bitmaps, struct btrfs_free_space, list);
+
+ if (!entry || entry->offset != bitmap_offset) {
entry = tree_search_offset(ctl, bitmap_offset, 1, 0);
if (entry && list_empty(&entry->list))
list_add(&entry->list, bitmaps);
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: Btrfs: check for empty bitmap list in setup_cluster_bitmaps
2015-12-15 17:08 Btrfs: check for empty bitmap list in setup_cluster_bitmaps Chris Mason
@ 2015-12-15 18:37 ` Josef Bacik
2015-12-15 19:59 ` Chris Mason
2015-12-16 1:48 ` Manish
1 sibling, 1 reply; 4+ messages in thread
From: Josef Bacik @ 2015-12-15 18:37 UTC (permalink / raw)
To: Chris Mason, Btrfs mailing list, Andrey Ryabinin, Dave Jones
On 12/15/2015 12:08 PM, Chris Mason wrote:
> Dave Jones found a warning from kasan in setup_cluster_bitmaps()
>
> ==================================================================
> BUG: KASAN: stack-out-of-bounds in setup_cluster_bitmap+0xc4/0x5a0 at
> addr ffff88039bef6828
> Read of size 8 by task nfsd/1009
> page:ffffea000e6fbd80 count:0 mapcount:0 mapping: (null)
> index:0x0
> flags: 0x8000000000000000()
> page dumped because: kasan: bad access detected
> CPU: 1 PID: 1009 Comm: nfsd Tainted: G W
> 4.4.0-rc3-backup-debug+ #1
> ffff880065647b50 000000006bb712c2 ffff88039bef6640 ffffffffa680a43e
> 0000004559c00000 ffff88039bef66c8 ffffffffa62638d1 ffffffffa61121c0
> ffff8803a5769de8 0000000000000296 ffff8803a5769df0 0000000000046280
> Call Trace:
> [<ffffffffa680a43e>] dump_stack+0x4b/0x6d
> [<ffffffffa62638d1>] kasan_report_error+0x501/0x520
> [<ffffffffa61121c0>] ? debug_show_all_locks+0x1e0/0x1e0
> [<ffffffffa6263948>] kasan_report+0x58/0x60
> [<ffffffffa6814b00>] ? rb_last+0x10/0x40
> [<ffffffffa66f8af4>] ? setup_cluster_bitmap+0xc4/0x5a0
> [<ffffffffa6262ead>] __asan_load8+0x5d/0x70
> [<ffffffffa66f8af4>] setup_cluster_bitmap+0xc4/0x5a0
> [<ffffffffa66f675a>] ? setup_cluster_no_bitmap+0x6a/0x400
> [<ffffffffa66fcd16>] btrfs_find_space_cluster+0x4b6/0x640
> [<ffffffffa66fc860>] ? btrfs_alloc_from_cluster+0x4e0/0x4e0
> [<ffffffffa66fc36e>] ? btrfs_return_cluster_to_free_space+0x9e/0xb0
> [<ffffffffa702dc37>] ? _raw_spin_unlock+0x27/0x40
> [<ffffffffa666a1a1>] find_free_extent+0xba1/0x1520
>
> Andrey noticed this was because we were doing list_first_entry on a list
> that might be empty. Rework the tests a bit so we don't do that.
>
> Signed-off-by: Chris Mason <clm@fb.com>
> Reprorted-by: Andrey Ryabinin <ryabinin.a.a@gmail.com>
> Reported-by: Dave Jones <dsj@fb.com>
>
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index 0948d34..e6fc7d9 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -2972,7 +2972,7 @@ setup_cluster_bitmap(struct btrfs_block_group_cache *block_group,
> u64 cont1_bytes, u64 min_bytes)
> {
> struct btrfs_free_space_ctl *ctl = block_group->free_space_ctl;
> - struct btrfs_free_space *entry;
> + struct btrfs_free_space *entry = NULL;
> int ret = -ENOSPC;
> u64 bitmap_offset = offset_to_bitmap(ctl, offset);
>
> @@ -2983,8 +2983,10 @@ setup_cluster_bitmap(struct btrfs_block_group_cache *block_group,
> * The bitmap that covers offset won't be in the list unless offset
> * is just its start offset.
> */
Just above this we have a if (ctl->total_bitmaps == 0) return NULL;
check that should make this useless, which means we're screwing up our
ctl->total_bitmaps counter somehow. We should probably figure out why
that is happening. Thanks,
Josef
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Btrfs: check for empty bitmap list in setup_cluster_bitmaps
2015-12-15 18:37 ` Josef Bacik
@ 2015-12-15 19:59 ` Chris Mason
0 siblings, 0 replies; 4+ messages in thread
From: Chris Mason @ 2015-12-15 19:59 UTC (permalink / raw)
To: Josef Bacik; +Cc: Btrfs mailing list, Andrey Ryabinin, Dave Jones
On Tue, Dec 15, 2015 at 01:37:01PM -0500, Josef Bacik wrote:
> On 12/15/2015 12:08 PM, Chris Mason wrote:
> >Dave Jones found a warning from kasan in setup_cluster_bitmaps()
> >
> >==================================================================
> >BUG: KASAN: stack-out-of-bounds in setup_cluster_bitmap+0xc4/0x5a0 at
> >addr ffff88039bef6828
> >Read of size 8 by task nfsd/1009
> >page:ffffea000e6fbd80 count:0 mapcount:0 mapping: (null)
> >index:0x0
> >flags: 0x8000000000000000()
> >page dumped because: kasan: bad access detected
> >CPU: 1 PID: 1009 Comm: nfsd Tainted: G W
> >4.4.0-rc3-backup-debug+ #1
> >ffff880065647b50 000000006bb712c2 ffff88039bef6640 ffffffffa680a43e
> >0000004559c00000 ffff88039bef66c8 ffffffffa62638d1 ffffffffa61121c0
> >ffff8803a5769de8 0000000000000296 ffff8803a5769df0 0000000000046280
> >Call Trace:
> >[<ffffffffa680a43e>] dump_stack+0x4b/0x6d
> >[<ffffffffa62638d1>] kasan_report_error+0x501/0x520
> >[<ffffffffa61121c0>] ? debug_show_all_locks+0x1e0/0x1e0
> >[<ffffffffa6263948>] kasan_report+0x58/0x60
> >[<ffffffffa6814b00>] ? rb_last+0x10/0x40
> >[<ffffffffa66f8af4>] ? setup_cluster_bitmap+0xc4/0x5a0
> >[<ffffffffa6262ead>] __asan_load8+0x5d/0x70
> >[<ffffffffa66f8af4>] setup_cluster_bitmap+0xc4/0x5a0
> >[<ffffffffa66f675a>] ? setup_cluster_no_bitmap+0x6a/0x400
> >[<ffffffffa66fcd16>] btrfs_find_space_cluster+0x4b6/0x640
> >[<ffffffffa66fc860>] ? btrfs_alloc_from_cluster+0x4e0/0x4e0
> >[<ffffffffa66fc36e>] ? btrfs_return_cluster_to_free_space+0x9e/0xb0
> >[<ffffffffa702dc37>] ? _raw_spin_unlock+0x27/0x40
> >[<ffffffffa666a1a1>] find_free_extent+0xba1/0x1520
> >
> >Andrey noticed this was because we were doing list_first_entry on a list
> >that might be empty. Rework the tests a bit so we don't do that.
> >
> >Signed-off-by: Chris Mason <clm@fb.com>
> >Reprorted-by: Andrey Ryabinin <ryabinin.a.a@gmail.com>
> >Reported-by: Dave Jones <dsj@fb.com>
> >
> >diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> >index 0948d34..e6fc7d9 100644
> >--- a/fs/btrfs/free-space-cache.c
> >+++ b/fs/btrfs/free-space-cache.c
> >@@ -2972,7 +2972,7 @@ setup_cluster_bitmap(struct btrfs_block_group_cache *block_group,
> > u64 cont1_bytes, u64 min_bytes)
> > {
> > struct btrfs_free_space_ctl *ctl = block_group->free_space_ctl;
> >- struct btrfs_free_space *entry;
> >+ struct btrfs_free_space *entry = NULL;
> > int ret = -ENOSPC;
> > u64 bitmap_offset = offset_to_bitmap(ctl, offset);
> >
> >@@ -2983,8 +2983,10 @@ setup_cluster_bitmap(struct btrfs_block_group_cache *block_group,
> > * The bitmap that covers offset won't be in the list unless offset
> > * is just its start offset.
> > */
>
> Just above this we have a if (ctl->total_bitmaps == 0) return NULL; check
> that should make this useless, which means we're screwing up our
> ctl->total_bitmaps counter somehow. We should probably figure out why that
> is happening. Thanks,
My best explanation is that btrfs_bitmap_cluster() takes the bitmap out
of the rbtree without dropping ctl->total_bitmaps. So,
setup_cluster_no_bitmap() can't find it. This should require mixed
allocation modes to trigger.
Another path is that during btrfs_write_out_cache() we'll pull entries
out. My relatively new code allows that to happen before commit now, so
it might happen then.
-chris
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Btrfs: check for empty bitmap list in setup_cluster_bitmaps
2015-12-15 17:08 Btrfs: check for empty bitmap list in setup_cluster_bitmaps Chris Mason
2015-12-15 18:37 ` Josef Bacik
@ 2015-12-16 1:48 ` Manish
1 sibling, 0 replies; 4+ messages in thread
From: Manish @ 2015-12-16 1:48 UTC (permalink / raw)
To: linux-btrfs
Hi Chris,
I have one coding comment for this patch.
Following line can be merged into single:
if (!list_empty(bitmaps))
entry = list_first_entry(bitmaps, struct btrfs_free_space, list);
new change as below-:
entry = list_first_entry_or_null(bitmaps, struct btrfs_free_space,list);
Manish
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-12-16 1:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-15 17:08 Btrfs: check for empty bitmap list in setup_cluster_bitmaps Chris Mason
2015-12-15 18:37 ` Josef Bacik
2015-12-15 19:59 ` Chris Mason
2015-12-16 1:48 ` Manish
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.