diff mbox series

[2/2] btrfs: tree-checker: check item_size for dev_item

Message ID 20220121093335.1840306-3-l@damenly.su (mailing list archive)
State New, archived
Headers show
Series Simple two patches for tree checker | expand

Commit Message

Su Yue Jan. 21, 2022, 9:33 a.m. UTC
Check item size before accessing the device item to avoid out of bound
access.

Signed-off-by: Su Yue <l@damenly.su>
---
 fs/btrfs/tree-checker.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Wang Yugui Feb. 5, 2022, 3:13 a.m. UTC | #1
Hi,

A btrfs filesystem failed to boot with this patch.

corrupt leaf: root=3 block=1081344 slot=0 devid=1 invalid item size: has 0 expect 98

Any way to fix it online?

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/02/05

> Check item size before accessing the device item to avoid out of bound
> access.
> 
> Signed-off-by: Su Yue <l@damenly.su>
> ---
>  fs/btrfs/tree-checker.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 2978fc89c093..87fff4345977 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -965,6 +965,7 @@ static int check_dev_item(struct extent_buffer *leaf,
>  			  struct btrfs_key *key, int slot)
>  {
>  	struct btrfs_dev_item *ditem;
> +	u32 item_size = btrfs_item_size(leaf, slot);
>  
>  	if (unlikely(key->objectid != BTRFS_DEV_ITEMS_OBJECTID)) {
>  		dev_item_err(leaf, slot,
> @@ -972,6 +973,13 @@ static int check_dev_item(struct extent_buffer *leaf,
>  			     key->objectid, BTRFS_DEV_ITEMS_OBJECTID);
>  		return -EUCLEAN;
>  	}
> +
> +	if (unlikely(item_size != sizeof(*ditem))) {
> +		dev_item_err(leaf, slot, "invalid item size: has=%u expect=%zu",
> +			     item_size, sizeof(*ditem));
> +		return -EUCLEAN;
> +	}
> +
>  	ditem = btrfs_item_ptr(leaf, slot, struct btrfs_dev_item);
>  	if (unlikely(btrfs_device_id(leaf, ditem) != key->offset)) {
>  		dev_item_err(leaf, slot,
> -- 
> 2.34.1
Wang Yugui Feb. 5, 2022, 4:35 a.m. UTC | #2
Hi,

> A btrfs filesystem failed to boot with this patch.
> 
> corrupt leaf: root=3 block=1081344 slot=0 devid=1 invalid item size: has 0 expect 98
> 
> Any way to fix it online?

This btrfs filesystem is created by centos 7.9 installer (btrfs 4.9?)
about 1 years ago.  and then mainly writen by kernel 5.4/5.10/5.15.

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/02/05


> 
> > Check item size before accessing the device item to avoid out of bound
> > access.
> > 
> > Signed-off-by: Su Yue <l@damenly.su>
> > ---
> >  fs/btrfs/tree-checker.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> > index 2978fc89c093..87fff4345977 100644
> > --- a/fs/btrfs/tree-checker.c
> > +++ b/fs/btrfs/tree-checker.c
> > @@ -965,6 +965,7 @@ static int check_dev_item(struct extent_buffer *leaf,
> >  			  struct btrfs_key *key, int slot)
> >  {
> >  	struct btrfs_dev_item *ditem;
> > +	u32 item_size = btrfs_item_size(leaf, slot);
> >  
> >  	if (unlikely(key->objectid != BTRFS_DEV_ITEMS_OBJECTID)) {
> >  		dev_item_err(leaf, slot,
> > @@ -972,6 +973,13 @@ static int check_dev_item(struct extent_buffer *leaf,
> >  			     key->objectid, BTRFS_DEV_ITEMS_OBJECTID);
> >  		return -EUCLEAN;
> >  	}
> > +
> > +	if (unlikely(item_size != sizeof(*ditem))) {
> > +		dev_item_err(leaf, slot, "invalid item size: has=%u expect=%zu",
> > +			     item_size, sizeof(*ditem));
> > +		return -EUCLEAN;
> > +	}
> > +
> >  	ditem = btrfs_item_ptr(leaf, slot, struct btrfs_dev_item);
> >  	if (unlikely(btrfs_device_id(leaf, ditem) != key->offset)) {
> >  		dev_item_err(leaf, slot,
> > -- 
> > 2.34.1
>
Su Yue Feb. 5, 2022, 11:15 a.m. UTC | #3
On Sat 05 Feb 2022 at 12:35, Wang Yugui <wangyugui@e16-tech.com> 
wrote:

> Hi,
>
>> A btrfs filesystem failed to boot with this patch.
>>
>> corrupt leaf: root=3 block=1081344 slot=0 devid=1 invalid item 
>> size: has 0 expect 98
>>
>> Any way to fix it online?
>
> This btrfs filesystem is created by centos 7.9 installer (btrfs 
> 4.9?)
> about 1 years ago.  and then mainly writen by kernel 
> 5.4/5.10/5.15.
>
Yes, btrfs-progs v4.9 and v3.10 based kernel.
I created a btrfs and it looks fine.
Could please provide output of
btrfs inspect-internal dump-tree $device -t 3
?
You can trim it if the content is too long only leaf 1081344 is 
needed.


--
Su

> Best Regards
> Wang Yugui (wangyugui@e16-tech.com)
> 2022/02/05
>
>
>>
>> > Check item size before accessing the device item to avoid out 
>> > of bound
>> > access.
>> >
>> > Signed-off-by: Su Yue <l@damenly.su>
>> > ---
>> >  fs/btrfs/tree-checker.c | 8 ++++++++
>> >  1 file changed, 8 insertions(+)
>> >
>> > diff --git a/fs/btrfs/tree-checker.c 
>> > b/fs/btrfs/tree-checker.c
>> > index 2978fc89c093..87fff4345977 100644
>> > --- a/fs/btrfs/tree-checker.c
>> > +++ b/fs/btrfs/tree-checker.c
>> > @@ -965,6 +965,7 @@ static int check_dev_item(struct 
>> > extent_buffer *leaf,
>> >  			  struct btrfs_key *key, int slot)
>> >  {
>> >  	struct btrfs_dev_item *ditem;
>> > +	u32 item_size = btrfs_item_size(leaf, slot);
>> >
>> >  	if (unlikely(key->objectid != BTRFS_DEV_ITEMS_OBJECTID)) {
>> >  		dev_item_err(leaf, slot,
>> > @@ -972,6 +973,13 @@ static int check_dev_item(struct 
>> > extent_buffer *leaf,
>> >  			     key->objectid, 
>> >  BTRFS_DEV_ITEMS_OBJECTID);
>> >  		return -EUCLEAN;
>> >  	}
>> > +
>> > +	if (unlikely(item_size != sizeof(*ditem))) {
>> > +		dev_item_err(leaf, slot, "invalid item size: 
>> > has=%u expect=%zu",
>> > +			     item_size, sizeof(*ditem));
>> > +		return -EUCLEAN;
>> > +	}
>> > +
>> >  	ditem = btrfs_item_ptr(leaf, slot, struct btrfs_dev_item);
>> >  	if (unlikely(btrfs_device_id(leaf, ditem) != key->offset)) 
>> >  {
>> >  		dev_item_err(leaf, slot,
>> > --
>> > 2.34.1
>>
Wang Yugui Feb. 5, 2022, 12:30 p.m. UTC | #4
Hi,

> >> A btrfs filesystem failed to boot with this patch.
> >>
> >> corrupt leaf: root=3 block=1081344 slot=0 devid=1 invalid item
> >> size: has 0 expect 98
> >>
> >> Any way to fix it online?
> >
> > This btrfs filesystem is created by centos 7.9 installer (btrfs
> > 4.9?)
> > about 1 years ago.  and then mainly writen by kernel
> > 5.4/5.10/5.15.
> >
> Yes, btrfs-progs v4.9 and v3.10 based kernel.
> I created a btrfs and it looks fine.
> Could please provide output of
> btrfs inspect-internal dump-tree $device -t 3
> ?
> You can trim it if the content is too long only leaf 1081344 is needed.

Hi,

# btrfs filesystem show /
Label: 'OS_T640'  uuid: 73dcce98-8f6b-4ec8-bfac-fa7c7c87409d
        Total devices 10 FS bytes used 5.53TiB
        devid    1 size 799.00GiB used 332.01GiB path /dev/sda2
        devid    2 size 1.75TiB used 741.00GiB path /dev/sdg1
        devid    3 size 1.75TiB used 745.00GiB path /dev/sdj1
        devid    4 size 1.75TiB used 740.00GiB path /dev/sdi1
        devid    5 size 1.75TiB used 745.00GiB path /dev/sdd1
        devid    6 size 1.75TiB used 480.00GiB path /dev/sde1
        devid    7 size 1.75TiB used 480.00GiB path /dev/sdh1
        devid    8 size 1.75TiB used 479.00GiB path /dev/sdc1
        devid    9 size 1.75TiB used 480.00GiB path /dev/sdb1
        devid   10 size 1.75TiB used 479.00GiB path /dev/sdf1

#btrfs inspect-internal dump-tree /dev/sda2 -t 3 > 3.txt

and then 3.txt is zipped as  this attachment file(3.zip)

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/02/05
Qu Wenruo Feb. 5, 2022, 1:01 p.m. UTC | #5
On 2022/2/5 20:30, Wang Yugui wrote:
> Hi,
>
>>>> A btrfs filesystem failed to boot with this patch.
>>>>
>>>> corrupt leaf: root=3 block=1081344 slot=0 devid=1 invalid item
>>>> size: has 0 expect 98
>>>>
>>>> Any way to fix it online?
>>>
>>> This btrfs filesystem is created by centos 7.9 installer (btrfs
>>> 4.9?)
>>> about 1 years ago.  and then mainly writen by kernel
>>> 5.4/5.10/5.15.
>>>
>> Yes, btrfs-progs v4.9 and v3.10 based kernel.
>> I created a btrfs and it looks fine.
>> Could please provide output of
>> btrfs inspect-internal dump-tree $device -t 3
>> ?
>> You can trim it if the content is too long only leaf 1081344 is needed.
>
> Hi,
>
> # btrfs filesystem show /
> Label: 'OS_T640'  uuid: 73dcce98-8f6b-4ec8-bfac-fa7c7c87409d
>          Total devices 10 FS bytes used 5.53TiB
>          devid    1 size 799.00GiB used 332.01GiB path /dev/sda2
>          devid    2 size 1.75TiB used 741.00GiB path /dev/sdg1
>          devid    3 size 1.75TiB used 745.00GiB path /dev/sdj1
>          devid    4 size 1.75TiB used 740.00GiB path /dev/sdi1
>          devid    5 size 1.75TiB used 745.00GiB path /dev/sdd1
>          devid    6 size 1.75TiB used 480.00GiB path /dev/sde1
>          devid    7 size 1.75TiB used 480.00GiB path /dev/sdh1
>          devid    8 size 1.75TiB used 479.00GiB path /dev/sdc1
>          devid    9 size 1.75TiB used 480.00GiB path /dev/sdb1
>          devid   10 size 1.75TiB used 479.00GiB path /dev/sdf1
>
> #btrfs inspect-internal dump-tree /dev/sda2 -t 3 > 3.txt
>
> and then 3.txt is zipped as  this attachment file(3.zip)

Full dmesg of the boot failure please.

The dump-tree shows the device item is completely sane, it has size 98,
not the value (0) reported from tree-checker.

Thus I don't know why tree-checker is reporting this problem.

Thanks,
Qu
>
> Best Regards
> Wang Yugui (wangyugui@e16-tech.com)
> 2022/02/05
>
>
Wang Yugui Feb. 5, 2022, 2:49 p.m. UTC | #6
Hi,

> >>>> A btrfs filesystem failed to boot with this patch.
> >>>>
> >>>> corrupt leaf: root=3 block=1081344 slot=0 devid=1 invalid item
> >>>> size: has 0 expect 98
> >>>>
> >>>> Any way to fix it online?
> >>>
> >>> This btrfs filesystem is created by centos 7.9 installer (btrfs
> >>> 4.9?)
> >>> about 1 years ago.  and then mainly writen by kernel
> >>> 5.4/5.10/5.15.
> >>>
> >> Yes, btrfs-progs v4.9 and v3.10 based kernel.
> >> I created a btrfs and it looks fine.
> >> Could please provide output of
> >> btrfs inspect-internal dump-tree $device -t 3
> >> ?
> >> You can trim it if the content is too long only leaf 1081344 is needed.
> >
> > Hi,
> >
> > # btrfs filesystem show /
> > Label: 'OS_T640'  uuid: 73dcce98-8f6b-4ec8-bfac-fa7c7c87409d
> >          Total devices 10 FS bytes used 5.53TiB
> >          devid    1 size 799.00GiB used 332.01GiB path /dev/sda2
> >          devid    2 size 1.75TiB used 741.00GiB path /dev/sdg1
> >          devid    3 size 1.75TiB used 745.00GiB path /dev/sdj1
> >          devid    4 size 1.75TiB used 740.00GiB path /dev/sdi1
> >          devid    5 size 1.75TiB used 745.00GiB path /dev/sdd1
> >          devid    6 size 1.75TiB used 480.00GiB path /dev/sde1
> >          devid    7 size 1.75TiB used 480.00GiB path /dev/sdh1
> >          devid    8 size 1.75TiB used 479.00GiB path /dev/sdc1
> >          devid    9 size 1.75TiB used 480.00GiB path /dev/sdb1
> >          devid   10 size 1.75TiB used 479.00GiB path /dev/sdf1
> >
> > #btrfs inspect-internal dump-tree /dev/sda2 -t 3 > 3.txt
> >
> > and then 3.txt is zipped as  this attachment file(3.zip)
> 
> Full dmesg of the boot failure please.
> 
> The dump-tree shows the device item is completely sane, it has size 98,
> not the value (0) reported from tree-checker.
> 
> Thus I don't know why tree-checker is reporting this problem.
> 

This (attachment file boot.dmesg.txt.zip ) is the full dmesg output

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/02/05
Su Yue Feb. 6, 2022, 12:12 p.m. UTC | #7
On Sat 05 Feb 2022 at 22:49, Wang Yugui <wangyugui@e16-tech.com> 
wrote:

> Hi,
>
>> >>>> A btrfs filesystem failed to boot with this patch.
>> >>>>
>> >>>> corrupt leaf: root=3 block=1081344 slot=0 devid=1 invalid 
>> >>>> item
>> >>>> size: has 0 expect 98
>> >>>>
>> >>>> Any way to fix it online?
>> >>>
>> >>> This btrfs filesystem is created by centos 7.9 installer 
>> >>> (btrfs
>> >>> 4.9?)
>> >>> about 1 years ago.  and then mainly writen by kernel
>> >>> 5.4/5.10/5.15.
>> >>>
>> >> Yes, btrfs-progs v4.9 and v3.10 based kernel.
>> >> I created a btrfs and it looks fine.
>> >> Could please provide output of
>> >> btrfs inspect-internal dump-tree $device -t 3
>> >> ?
>> >> You can trim it if the content is too long only leaf 1081344 
>> >> is needed.
>> >
>> > Hi,
>> >
>> > # btrfs filesystem show /
>> > Label: 'OS_T640'  uuid: 73dcce98-8f6b-4ec8-bfac-fa7c7c87409d
>> >          Total devices 10 FS bytes used 5.53TiB
>> >          devid    1 size 799.00GiB used 332.01GiB path 
>> >          /dev/sda2
>> >          devid    2 size 1.75TiB used 741.00GiB path 
>> >          /dev/sdg1
>> >          devid    3 size 1.75TiB used 745.00GiB path 
>> >          /dev/sdj1
>> >          devid    4 size 1.75TiB used 740.00GiB path 
>> >          /dev/sdi1
>> >          devid    5 size 1.75TiB used 745.00GiB path 
>> >          /dev/sdd1
>> >          devid    6 size 1.75TiB used 480.00GiB path 
>> >          /dev/sde1
>> >          devid    7 size 1.75TiB used 480.00GiB path 
>> >          /dev/sdh1
>> >          devid    8 size 1.75TiB used 479.00GiB path 
>> >          /dev/sdc1
>> >          devid    9 size 1.75TiB used 480.00GiB path 
>> >          /dev/sdb1
>> >          devid   10 size 1.75TiB used 479.00GiB path 
>> >          /dev/sdf1
>> >
>> > #btrfs inspect-internal dump-tree /dev/sda2 -t 3 > 3.txt
>> >
>> > and then 3.txt is zipped as  this attachment file(3.zip)
>>
>> Full dmesg of the boot failure please.
>>
>> The dump-tree shows the device item is completely sane, it has 
>> size 98,
>> not the value (0) reported from tree-checker.
>>
>> Thus I don't know why tree-checker is reporting this problem.
>>
>
> This (attachment file boot.dmesg.txt.zip ) is the full dmesg 
> output
>
As Qu suggested to me, would you plase provide output after
apply of the following diff? (It may crash the kernel if there is 
*real*
one invalid dev_item).

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 9fd145f1c4bc..5fb981b4b42a 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -25,6 +25,7 @@
 #include "volumes.h"
 #include "misc.h"
 #include "btrfs_inode.h"
+#include "print-tree.h"

 /*
  * Error message should follow the following format:
@@ -977,6 +978,7 @@ static int check_dev_item(struct extent_buffer 
*leaf,
        if (unlikely(item_size != sizeof(*ditem))) {
                dev_item_err(leaf, slot, "invalid item size: has 
                %u expect %zu",
                             item_size, sizeof(*ditem));
+               btrfs_print_leaf(leaf);
                return -EUCLEAN;
        }


--
Su
> Best Regards
> Wang Yugui (wangyugui@e16-tech.com)
> 2022/02/05
>
> [2. application/x-zip-compressed; boot.dmesg.txt.zip]...
Wang Yugui Feb. 6, 2022, 3:39 p.m. UTC | #8
Hi,

> >> >>>> A btrfs filesystem failed to boot with this patch.
> >> >>>>
> >> >>>> corrupt leaf: root=3 block=1081344 slot=0 devid=1 invalid
> >> >>>> item
> >> >>>> size: has 0 expect 98
> >> >>>>


> As Qu suggested to me, would you plase provide output after
> apply of the following diff? (It may crash the kernel if there is *real*
> one invalid dev_item).
> 
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 9fd145f1c4bc..5fb981b4b42a 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -25,6 +25,7 @@
>  #include "volumes.h"
>  #include "misc.h"
>  #include "btrfs_inode.h"
> +#include "print-tree.h"
> 
>  /*
>   * Error message should follow the following format:
> @@ -977,6 +978,7 @@ static int check_dev_item(struct extent_buffer *leaf,
>         if (unlikely(item_size != sizeof(*ditem))) {
>                 dev_item_err(leaf, slot, "invalid item size: has                 %u expect %zu",
>                              item_size, sizeof(*ditem));
> +               btrfs_print_leaf(leaf);
>                 return -EUCLEAN;
>         }

When I tested this new diag patch, I noticed that I wrongly applied
these 2 patches to 5.15.x.
btrfs-tree-checker-check-item_size-for-inode_item.patch
btrfs-tree-checker-check-item_size-for-dev_item.patch

some depency patches(at least btrfs-drop-the-_nr-from-the-item-helpers.patch,
maybe more) are missed.

In fact, without these depency patches, there are some build warning,
but I failed to noticed them.

so this is just my bad now. sorry.

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/02/06
diff mbox series

Patch

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 2978fc89c093..87fff4345977 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -965,6 +965,7 @@  static int check_dev_item(struct extent_buffer *leaf,
 			  struct btrfs_key *key, int slot)
 {
 	struct btrfs_dev_item *ditem;
+	u32 item_size = btrfs_item_size(leaf, slot);
 
 	if (unlikely(key->objectid != BTRFS_DEV_ITEMS_OBJECTID)) {
 		dev_item_err(leaf, slot,
@@ -972,6 +973,13 @@  static int check_dev_item(struct extent_buffer *leaf,
 			     key->objectid, BTRFS_DEV_ITEMS_OBJECTID);
 		return -EUCLEAN;
 	}
+
+	if (unlikely(item_size != sizeof(*ditem))) {
+		dev_item_err(leaf, slot, "invalid item size: has=%u expect=%zu",
+			     item_size, sizeof(*ditem));
+		return -EUCLEAN;
+	}
+
 	ditem = btrfs_item_ptr(leaf, slot, struct btrfs_dev_item);
 	if (unlikely(btrfs_device_id(leaf, ditem) != key->offset)) {
 		dev_item_err(leaf, slot,