Linux Btrfs filesystem development
 help / color / mirror / Atom feed
* [bug report] btrfs: fix corrupt read due to bad offset of a compressed extent map
@ 2026-06-15 12:46 Dan Carpenter
  2026-06-15 12:56 ` Filipe Manana
  0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2026-06-15 12:46 UTC (permalink / raw)
  To: Filipe Manana; +Cc: linux-btrfs

Hello Filipe Manana,

Commit de9f46cb0044 ("btrfs: fix corrupt read due to bad offset of a
compressed extent map") from Jul 11, 2024 (linux-next), leads to the
following Smatch static checker warning:

	fs/btrfs/tests/extent-map-tests.c:979 test_case_8()
	error: dereferencing freed memory 'em' (line 973 btrfs_free_extent_map())

fs/btrfs/tests/extent-map-tests.c
    912 static int test_case_8(struct btrfs_fs_info *fs_info, struct btrfs_inode *inode)
    913 {
    914         struct extent_map_tree *em_tree = &inode->extent_tree;
    915         struct extent_map *em;
    916         int ret;
    917         int ret2;
    918 
    919         em = btrfs_alloc_extent_map();
    920         if (!em) {
    921                 test_std_err(TEST_ALLOC_EXTENT_MAP);
    922                 return -ENOMEM;
    923         }
    924 
    925         /* Compressed extent for the file range [120K, 128K). */
    926         em->start = SZ_1K * 120;
    927         em->len = SZ_8K;
    928         em->disk_num_bytes = SZ_4K;
    929         em->ram_bytes = SZ_8K;
    930         em->flags |= EXTENT_FLAG_COMPRESS_ZLIB;
    931         write_lock(&em_tree->lock);
    932         ret = btrfs_add_extent_mapping(inode, &em, em->start, em->len);
    933         write_unlock(&em_tree->lock);
    934         btrfs_free_extent_map(em);
    935         if (ret < 0) {
    936                 test_err("couldn't add extent map for range [120K, 128K)");
    937                 goto out;
    938         }
    939 
    940         em = btrfs_alloc_extent_map();
    941         if (!em) {
    942                 test_std_err(TEST_ALLOC_EXTENT_MAP);
    943                 ret = -ENOMEM;
    944                 goto out;
    945         }
    946 
    947         /*
    948          * Compressed extent for the file range [108K, 144K), which overlaps
    949          * with the [120K, 128K) we previously inserted.
    950          */
    951         em->start = SZ_1K * 108;
    952         em->len = SZ_1K * 36;
    953         em->disk_num_bytes = SZ_4K;
    954         em->ram_bytes = SZ_1K * 36;
    955         em->flags |= EXTENT_FLAG_COMPRESS_ZLIB;
    956 
    957         /*
    958          * Try to add the extent map but with a search range of [140K, 144K),
    959          * this should succeed and adjust the extent map to the range
    960          * [128K, 144K), with a length of 16K and an offset of 20K.
    961          *
    962          * This simulates a scenario where in the subvolume tree of an inode we
    963          * have a compressed file extent item for the range [108K, 144K) and we
    964          * have an overlapping compressed extent map for the range [120K, 128K),
    965          * which was created by an encoded write, but its ordered extent was not
    966          * yet completed, so the subvolume tree doesn't have yet the file extent
    967          * item for that range - we only have the extent map in the inode's
    968          * extent map tree.
    969          */
    970         write_lock(&em_tree->lock);
    971         ret = btrfs_add_extent_mapping(inode, &em, SZ_1K * 140, SZ_4K);
    972         write_unlock(&em_tree->lock);
    973         btrfs_free_extent_map(em);

This looks like btrfs_free_extent_map() frees "em".

    974         if (ret < 0) {
    975                 test_err("couldn't add extent map for range [108K, 144K)");
    976                 goto out;
    977         }
    978 
--> 979         if (em->start != SZ_128K) {
                    ^^^^^^^^^

    980                 test_err("unexpected extent map start %llu (should be 128K)", em->start);
                                                                                      ^^^^^^^^^
    981                 ret = -EINVAL;
    982                 goto out;
    983         }
    984         if (em->len != SZ_16K) {
                    ^^^^^^^

    985                 test_err("unexpected extent map length %llu (should be 16K)", em->len);
                                                                                      ^^^^^^^
    986                 ret = -EINVAL;
    987                 goto out;
    988         }
    989         if (em->offset != SZ_1K * 20) {
                    ^^^^^^^^^^
    990                 test_err("unexpected extent map offset %llu (should be 20K)", em->offset);
                                                                                      ^^^^^^^^^^
    991                 ret = -EINVAL;
    992                 goto out;
    993         }
    994 out:
    995         ret2 = free_extent_map_tree(inode);
    996         if (ret == 0)
    997                 ret = ret2;
    998 
    999         return ret;
    1000 }

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [bug report] btrfs: fix corrupt read due to bad offset of a compressed extent map
  2026-06-15 12:46 [bug report] btrfs: fix corrupt read due to bad offset of a compressed extent map Dan Carpenter
@ 2026-06-15 12:56 ` Filipe Manana
  2026-06-15 13:12   ` Dan Carpenter
  0 siblings, 1 reply; 3+ messages in thread
From: Filipe Manana @ 2026-06-15 12:56 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Filipe Manana, linux-btrfs

On Mon, Jun 15, 2026 at 1:47 PM Dan Carpenter <error27@gmail.com> wrote:
>
> Hello Filipe Manana,
>
> Commit de9f46cb0044 ("btrfs: fix corrupt read due to bad offset of a
> compressed extent map") from Jul 11, 2024 (linux-next), leads to the
> following Smatch static checker warning:
>
>         fs/btrfs/tests/extent-map-tests.c:979 test_case_8()
>         error: dereferencing freed memory 'em' (line 973 btrfs_free_extent_map())
>
> fs/btrfs/tests/extent-map-tests.c
>     912 static int test_case_8(struct btrfs_fs_info *fs_info, struct btrfs_inode *inode)
>     913 {
>     914         struct extent_map_tree *em_tree = &inode->extent_tree;
>     915         struct extent_map *em;
>     916         int ret;
>     917         int ret2;
>     918
>     919         em = btrfs_alloc_extent_map();
>     920         if (!em) {
>     921                 test_std_err(TEST_ALLOC_EXTENT_MAP);
>     922                 return -ENOMEM;
>     923         }
>     924
>     925         /* Compressed extent for the file range [120K, 128K). */
>     926         em->start = SZ_1K * 120;
>     927         em->len = SZ_8K;
>     928         em->disk_num_bytes = SZ_4K;
>     929         em->ram_bytes = SZ_8K;
>     930         em->flags |= EXTENT_FLAG_COMPRESS_ZLIB;
>     931         write_lock(&em_tree->lock);
>     932         ret = btrfs_add_extent_mapping(inode, &em, em->start, em->len);
>     933         write_unlock(&em_tree->lock);
>     934         btrfs_free_extent_map(em);
>     935         if (ret < 0) {
>     936                 test_err("couldn't add extent map for range [120K, 128K)");
>     937                 goto out;
>     938         }
>     939
>     940         em = btrfs_alloc_extent_map();
>     941         if (!em) {
>     942                 test_std_err(TEST_ALLOC_EXTENT_MAP);
>     943                 ret = -ENOMEM;
>     944                 goto out;
>     945         }
>     946
>     947         /*
>     948          * Compressed extent for the file range [108K, 144K), which overlaps
>     949          * with the [120K, 128K) we previously inserted.
>     950          */
>     951         em->start = SZ_1K * 108;
>     952         em->len = SZ_1K * 36;
>     953         em->disk_num_bytes = SZ_4K;
>     954         em->ram_bytes = SZ_1K * 36;
>     955         em->flags |= EXTENT_FLAG_COMPRESS_ZLIB;
>     956
>     957         /*
>     958          * Try to add the extent map but with a search range of [140K, 144K),
>     959          * this should succeed and adjust the extent map to the range
>     960          * [128K, 144K), with a length of 16K and an offset of 20K.
>     961          *
>     962          * This simulates a scenario where in the subvolume tree of an inode we
>     963          * have a compressed file extent item for the range [108K, 144K) and we
>     964          * have an overlapping compressed extent map for the range [120K, 128K),
>     965          * which was created by an encoded write, but its ordered extent was not
>     966          * yet completed, so the subvolume tree doesn't have yet the file extent
>     967          * item for that range - we only have the extent map in the inode's
>     968          * extent map tree.
>     969          */
>     970         write_lock(&em_tree->lock);
>     971         ret = btrfs_add_extent_mapping(inode, &em, SZ_1K * 140, SZ_4K);
>     972         write_unlock(&em_tree->lock);
>     973         btrfs_free_extent_map(em);
>
> This looks like btrfs_free_extent_map() frees "em".

Nop, false alarm.
And that's because btrfs_add_extent_mapping() will increase the ref
count of the extent map if it returns success (one ref for the tree).
So until the extent map is removed from the tree, we can use the em
after calling btrfs_free_extent_map().

Thanks.

>
>     974         if (ret < 0) {
>     975                 test_err("couldn't add extent map for range [108K, 144K)");
>     976                 goto out;
>     977         }
>     978
> --> 979         if (em->start != SZ_128K) {
>                     ^^^^^^^^^
>
>     980                 test_err("unexpected extent map start %llu (should be 128K)", em->start);
>                                                                                       ^^^^^^^^^
>     981                 ret = -EINVAL;
>     982                 goto out;
>     983         }
>     984         if (em->len != SZ_16K) {
>                     ^^^^^^^
>
>     985                 test_err("unexpected extent map length %llu (should be 16K)", em->len);
>                                                                                       ^^^^^^^
>     986                 ret = -EINVAL;
>     987                 goto out;
>     988         }
>     989         if (em->offset != SZ_1K * 20) {
>                     ^^^^^^^^^^
>     990                 test_err("unexpected extent map offset %llu (should be 20K)", em->offset);
>                                                                                       ^^^^^^^^^^
>     991                 ret = -EINVAL;
>     992                 goto out;
>     993         }
>     994 out:
>     995         ret2 = free_extent_map_tree(inode);
>     996         if (ret == 0)
>     997                 ret = ret2;
>     998
>     999         return ret;
>     1000 }
>
> This email is a free service from the Smatch-CI project [smatch.sf.net].
>
> regards,
> dan carpenter
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [bug report] btrfs: fix corrupt read due to bad offset of a compressed extent map
  2026-06-15 12:56 ` Filipe Manana
@ 2026-06-15 13:12   ` Dan Carpenter
  0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2026-06-15 13:12 UTC (permalink / raw)
  To: Filipe Manana; +Cc: Filipe Manana, linux-btrfs

On Mon, Jun 15, 2026 at 01:56:22PM +0100, Filipe Manana wrote:
> On Mon, Jun 15, 2026 at 1:47 PM Dan Carpenter <error27@gmail.com> wrote:
> >     957         /*
> >     958          * Try to add the extent map but with a search range of [140K, 144K),
> >     959          * this should succeed and adjust the extent map to the range
> >     960          * [128K, 144K), with a length of 16K and an offset of 20K.
> >     961          *
> >     962          * This simulates a scenario where in the subvolume tree of an inode we
> >     963          * have a compressed file extent item for the range [108K, 144K) and we
> >     964          * have an overlapping compressed extent map for the range [120K, 128K),
> >     965          * which was created by an encoded write, but its ordered extent was not
> >     966          * yet completed, so the subvolume tree doesn't have yet the file extent
> >     967          * item for that range - we only have the extent map in the inode's
> >     968          * extent map tree.
> >     969          */
> >     970         write_lock(&em_tree->lock);
> >     971         ret = btrfs_add_extent_mapping(inode, &em, SZ_1K * 140, SZ_4K);
> >     972         write_unlock(&em_tree->lock);
> >     973         btrfs_free_extent_map(em);
> >
> > This looks like btrfs_free_extent_map() frees "em".
> 
> Nop, false alarm.
> And that's because btrfs_add_extent_mapping() will increase the ref
> count of the extent map if it returns success (one ref for the tree).
> So until the extent map is removed from the tree, we can use the em
> after calling btrfs_free_extent_map().
>

Ah.  Thanks for taking a look...

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-06-15 13:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-15 12:46 [bug report] btrfs: fix corrupt read due to bad offset of a compressed extent map Dan Carpenter
2026-06-15 12:56 ` Filipe Manana
2026-06-15 13:12   ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox