[Fix CVE-2024-50217 in v6.6.y] [PATCH] btrfs: fix use-after-free of block device file in __btrfs_free_extra_devids()

[Fix CVE-2024-50217 in v6.6.y] [PATCH] btrfs: fix use-after-free of block device file in __btrfs_free_extra_devids()

[Shubham Pushpkar]

From: Zhihao Cheng <chengzhihao1@huawei.com>

commit aec8e6bf839101784f3ef037dcdb9432c3f32343 ("btrfs:
fix use-after-free of block device file in __btrfs_free_extra_devids()")

Mounting btrfs from two images (which have the same one fsid and two
different dev_uuids) in certain executing order may trigger an UAF for
variable 'device->bdev_file' in __btrfs_free_extra_devids(). And
following are the details:

1. Attach image_1 to loop0, attach image_2 to loop1, and scan btrfs
   devices by ioctl(BTRFS_IOC_SCAN_DEV):

             /  btrfs_device_1 → loop0
   fs_device
             \  btrfs_device_2 → loop1
2. mount /dev/loop0 /mnt
   btrfs_open_devices
    btrfs_device_1->bdev_file = btrfs_get_bdev_and_sb(loop0)
    btrfs_device_2->bdev_file = btrfs_get_bdev_and_sb(loop1)
   btrfs_fill_super
    open_ctree
     fail: btrfs_close_devices // -ENOMEM
	    btrfs_close_bdev(btrfs_device_1)
             fput(btrfs_device_1->bdev_file)
	      // btrfs_device_1->bdev_file is freed
	    btrfs_close_bdev(btrfs_device_2)
             fput(btrfs_device_2->bdev_file)

3. mount /dev/loop1 /mnt
   btrfs_open_devices
    btrfs_get_bdev_and_sb(&bdev_file)
     // EIO, btrfs_device_1->bdev_file is not assigned,
     // which points to a freed memory area
    btrfs_device_2->bdev_file = btrfs_get_bdev_and_sb(loop1)
   btrfs_fill_super
    open_ctree
     btrfs_free_extra_devids
      if (btrfs_device_1->bdev_file)
       fput(btrfs_device_1->bdev_file) // UAF !

Fix it by setting 'device->bdev_file' as 'NULL' after closing the
btrfs_device in btrfs_close_one_device().

Fixes: CVE-2024-50217
Fixes: 142388194191 ("btrfs: do not background blkdev_put()")
CC: stable@vger.kernel.org # 4.19+
Link: https://bugzilla.kernel.org/show_bug.cgi?id=219408
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
(cherry picked from commit aec8e6bf839101784f3ef037dcdb9432c3f32343)
Signed-off-by: Shubham Pushpkar <spushpka@cisco.com>
---
 fs/btrfs/volumes.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index b9a0b26d08e1..ab2412542ce5 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1176,6 +1176,7 @@ static void btrfs_close_one_device(struct btrfs_device *device)
 	if (device->bdev) {
 		fs_devices->open_devices--;
 		device->bdev = NULL;
+		device->bdev_file = NULL;
 	}
 	clear_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
 	btrfs_destroy_dev_zone_info(device);
-- 
2.35.6

Re: [Fix CVE-2024-50217 in v6.6.y] [PATCH] btrfs: fix use-after-free of block device file in __btrfs_free_extra_devids()

[Greg KH]

On Mon, Feb 03, 2025 at 02:42:54AM -0800, Shubham Pushpkar wrote:
> From: Zhihao Cheng <chengzhihao1@huawei.com>
> 
> commit aec8e6bf839101784f3ef037dcdb9432c3f32343 ("btrfs:
> fix use-after-free of block device file in __btrfs_free_extra_devids()")
> 
> Mounting btrfs from two images (which have the same one fsid and two
> different dev_uuids) in certain executing order may trigger an UAF for
> variable 'device->bdev_file' in __btrfs_free_extra_devids(). And
> following are the details:
> 
> 1. Attach image_1 to loop0, attach image_2 to loop1, and scan btrfs
>    devices by ioctl(BTRFS_IOC_SCAN_DEV):
> 
>              /  btrfs_device_1 → loop0
>    fs_device
>              \  btrfs_device_2 → loop1
> 2. mount /dev/loop0 /mnt
>    btrfs_open_devices
>     btrfs_device_1->bdev_file = btrfs_get_bdev_and_sb(loop0)
>     btrfs_device_2->bdev_file = btrfs_get_bdev_and_sb(loop1)
>    btrfs_fill_super
>     open_ctree
>      fail: btrfs_close_devices // -ENOMEM
> 	    btrfs_close_bdev(btrfs_device_1)
>              fput(btrfs_device_1->bdev_file)
> 	      // btrfs_device_1->bdev_file is freed
> 	    btrfs_close_bdev(btrfs_device_2)
>              fput(btrfs_device_2->bdev_file)
> 
> 3. mount /dev/loop1 /mnt
>    btrfs_open_devices
>     btrfs_get_bdev_and_sb(&bdev_file)
>      // EIO, btrfs_device_1->bdev_file is not assigned,
>      // which points to a freed memory area
>     btrfs_device_2->bdev_file = btrfs_get_bdev_and_sb(loop1)
>    btrfs_fill_super
>     open_ctree
>      btrfs_free_extra_devids
>       if (btrfs_device_1->bdev_file)
>        fput(btrfs_device_1->bdev_file) // UAF !
> 
> Fix it by setting 'device->bdev_file' as 'NULL' after closing the
> btrfs_device in btrfs_close_one_device().
> 
> Fixes: CVE-2024-50217

Nit, as we assign CVEs _after_ a commit happens, there's no need to add
this to a commit here as it is implied by the assignment database of
cves-to-commits.

Also, any specific reason you didn't cc: everyone involved in this
commit for your backport as well?

thanks,

greg k-h

[Fix CVE-2024-50217 in v6.6.y] [PATCH] btrfs: fix use-after-free of block device file in __btrfs_free_extra_devids()

[Shubham Pushpkar]

From: Zhihao Cheng <chengzhihao1@huawei.com>

commit aec8e6bf839101784f3ef037dcdb9432c3f32343 ("btrfs:
fix use-after-free of block device file in __btrfs_free_extra_devids()")

Mounting btrfs from two images (which have the same one fsid and two
different dev_uuids) in certain executing order may trigger an UAF for
variable 'device->bdev_file' in __btrfs_free_extra_devids(). And
following are the details:

1. Attach image_1 to loop0, attach image_2 to loop1, and scan btrfs
   devices by ioctl(BTRFS_IOC_SCAN_DEV):

             /  btrfs_device_1 → loop0
   fs_device
             \  btrfs_device_2 → loop1
2. mount /dev/loop0 /mnt
   btrfs_open_devices
    btrfs_device_1->bdev_file = btrfs_get_bdev_and_sb(loop0)
    btrfs_device_2->bdev_file = btrfs_get_bdev_and_sb(loop1)
   btrfs_fill_super
    open_ctree
     fail: btrfs_close_devices // -ENOMEM
	    btrfs_close_bdev(btrfs_device_1)
             fput(btrfs_device_1->bdev_file)
	      // btrfs_device_1->bdev_file is freed
	    btrfs_close_bdev(btrfs_device_2)
             fput(btrfs_device_2->bdev_file)

3. mount /dev/loop1 /mnt
   btrfs_open_devices
    btrfs_get_bdev_and_sb(&bdev_file)
     // EIO, btrfs_device_1->bdev_file is not assigned,
     // which points to a freed memory area
    btrfs_device_2->bdev_file = btrfs_get_bdev_and_sb(loop1)
   btrfs_fill_super
    open_ctree
     btrfs_free_extra_devids
      if (btrfs_device_1->bdev_file)
       fput(btrfs_device_1->bdev_file) // UAF !

Fix it by setting 'device->bdev_file' as 'NULL' after closing the
btrfs_device in btrfs_close_one_device().

Fixes: 142388194191 ("btrfs: do not background blkdev_put()")
CC: stable@vger.kernel.org # 4.19+
Link: https://bugzilla.kernel.org/show_bug.cgi?id=219408
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
(cherry picked from commit aec8e6bf839101784f3ef037dcdb9432c3f32343)
Signed-off-by: Shubham Pushpkar <spushpka@cisco.com>
---
 fs/btrfs/volumes.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index b9a0b26d08e1..ab2412542ce5 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1176,6 +1176,7 @@ static void btrfs_close_one_device(struct btrfs_device *device)
 	if (device->bdev) {
 		fs_devices->open_devices--;
 		device->bdev = NULL;
+		device->bdev_file = NULL;
 	}
 	clear_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
 	btrfs_destroy_dev_zone_info(device);
-- 
2.35.6

Re: [Fix CVE-2024-50217 in v6.6.y] [PATCH] btrfs: fix use-after-free of block device file in __btrfs_free_extra_devids()

[Harshit Mogalapalli]

Hi Shubham,

On 03/02/25 18:07, Shubham Pushpkar wrote:
> From: Zhihao Cheng <chengzhihao1@huawei.com>
> 
> commit aec8e6bf839101784f3ef037dcdb9432c3f32343 ("btrfs:
> fix use-after-free of block device file in __btrfs_free_extra_devids()")
> 
> Mounting btrfs from two images (which have the same one fsid and two
> different dev_uuids) in certain executing order may trigger an UAF for
> variable 'device->bdev_file' in __btrfs_free_extra_devids(). And
> following are the details:
> 
> 1. Attach image_1 to loop0, attach image_2 to loop1, and scan btrfs
>     devices by ioctl(BTRFS_IOC_SCAN_DEV):
> 
>               /  btrfs_device_1 → loop0
>     fs_device
>               \  btrfs_device_2 → loop1
> 2. mount /dev/loop0 /mnt
>     btrfs_open_devices
>      btrfs_device_1->bdev_file = btrfs_get_bdev_and_sb(loop0)
>      btrfs_device_2->bdev_file = btrfs_get_bdev_and_sb(loop1)
>     btrfs_fill_super
>      open_ctree
>       fail: btrfs_close_devices // -ENOMEM
> 	    btrfs_close_bdev(btrfs_device_1)
>               fput(btrfs_device_1->bdev_file)
> 	      // btrfs_device_1->bdev_file is freed
> 	    btrfs_close_bdev(btrfs_device_2)
>               fput(btrfs_device_2->bdev_file)
> 
> 3. mount /dev/loop1 /mnt
>     btrfs_open_devices
>      btrfs_get_bdev_and_sb(&bdev_file)
>       // EIO, btrfs_device_1->bdev_file is not assigned,
>       // which points to a freed memory area
>      btrfs_device_2->bdev_file = btrfs_get_bdev_and_sb(loop1)
>     btrfs_fill_super
>      open_ctree
>       btrfs_free_extra_devids
>        if (btrfs_device_1->bdev_file)
>         fput(btrfs_device_1->bdev_file) // UAF !
> 
> Fix it by setting 'device->bdev_file' as 'NULL' after closing the
> btrfs_device in btrfs_close_one_device().
> 
> Fixes: 142388194191 ("btrfs: do not background blkdev_put()")
> CC: stable@vger.kernel.org # 4.19+
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=219408
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> Reviewed-by: David Sterba <dsterba@suse.com>
> Signed-off-by: David Sterba <dsterba@suse.com>
> (cherry picked from commit aec8e6bf839101784f3ef037dcdb9432c3f32343)
> Signed-off-by: Shubham Pushpkar <spushpka@cisco.com>
> ---
>   fs/btrfs/volumes.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index b9a0b26d08e1..ab2412542ce5 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1176,6 +1176,7 @@ static void btrfs_close_one_device(struct btrfs_device *device)
>   	if (device->bdev) {
>   		fs_devices->open_devices--;
>   		device->bdev = NULL;
> +		device->bdev_file = NULL;

Looks bad.

"bdev_file" is not a member of struct btrfs_device in 6.6.y. It is added 
only in commit: 9ae061cf2a46 ("btrfs: port device access to file") which 
is not in 6.6.y

Hint: This has CC:stable and Fixes tag and a clean cherry-pick so there 
should be a reason why this didn't get into stable easily.(reason in 
this case in build failure)

Thanks,
Harshit
>   	}
>   	clear_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
>   	btrfs_destroy_dev_zone_info(device);

Re: [Fix CVE-2024-50217 in v6.6.y] [PATCH] btrfs: fix use-after-free of block device file in __btrfs_free_extra_devids()

[Greg KH]

On Mon, Feb 03, 2025 at 11:40:11AM +0000, Shubham Pushpkar -X (spushpka - E INFOCHIPS PRIVATE LIMITED at Cisco) wrote:
> Hi Greg,
> 
> Thank you for your valuable feedback on my recent patch submission regarding the CVE fix.
> 
> I appreciate your point about the CVE reference in the commit message. I will revise the patch to remove the CVE identifier, as it is indeed managed in the assignment database.
> 
> Regarding the cc list, I apologize for not including everyone involved in the original commit. I will ensure to cc all relevant parties when I resubmit the patch to facilitate better communication and feedback.
> 
> Thank you once again for your guidance. Please let me know if there are any additional changes or considerations I should be aware of.

Yes, please always test your patches before sending them out.

Because of a lack of testing here, I'm going to have to ask you to get a
signed-off-by from another of your coworkers so that we know two people
have verified that the change is correct and actually works for any
future stuff you submit.

thanks,

greg k-h

Re: [Fix CVE-2024-50217 in v6.6.y] [PATCH] btrfs: fix use-after-free of block device file in __btrfs_free_extra_devids()

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

Found matching upstream commit: aec8e6bf839101784f3ef037dcdb9432c3f32343

WARNING: Author mismatch between patch and found commit:
Backport author: Shubham Pushpkar<spushpka@cisco.com>
Commit author: Zhihao Cheng<chengzhihao1@huawei.com>


Status in newer kernel trees:
6.13.y | Present (exact SHA1)
6.12.y | Present (exact SHA1)
6.6.y | Not found

Note: The patch differs from the upstream commit:
---
1:  aec8e6bf83910 ! 1:  afbc8c0c36536 btrfs: fix use-after-free of block device file in __btrfs_free_extra_devids()
    @@ Metadata
      ## Commit message ##
         btrfs: fix use-after-free of block device file in __btrfs_free_extra_devids()
     
    +    commit aec8e6bf839101784f3ef037dcdb9432c3f32343 ("btrfs:
    +    fix use-after-free of block device file in __btrfs_free_extra_devids()")
    +
         Mounting btrfs from two images (which have the same one fsid and two
         different dev_uuids) in certain executing order may trigger an UAF for
         variable 'device->bdev_file' in __btrfs_free_extra_devids(). And
    @@ Commit message
         Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
         Reviewed-by: David Sterba <dsterba@suse.com>
         Signed-off-by: David Sterba <dsterba@suse.com>
    +    (cherry picked from commit aec8e6bf839101784f3ef037dcdb9432c3f32343)
    +    Signed-off-by: Shubham Pushpkar <spushpka@cisco.com>
     
      ## fs/btrfs/volumes.c ##
     @@ fs/btrfs/volumes.c: static void btrfs_close_one_device(struct btrfs_device *device)
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.6.y        |  Success    |  Failed    |

Build Errors:
Build error for stable/linux-6.6.y:
    lib/test_dhry.o: warning: objtool: dhry() falls through to next function dhry_run_set.cold()
    fs/btrfs/volumes.c: In function 'btrfs_close_one_device':
    fs/btrfs/volumes.c:1179:23: error: 'struct btrfs_device' has no member named 'bdev_file'
     1179 |                 device->bdev_file = NULL;
          |                       ^~
    make[4]: *** [scripts/Makefile.build:243: fs/btrfs/volumes.o] Error 1
    make[4]: Target 'fs/btrfs/' not remade because of errors.
    make[3]: *** [scripts/Makefile.build:480: fs/btrfs] Error 2
    make[3]: Target 'fs/' not remade because of errors.
    make[2]: *** [scripts/Makefile.build:480: fs] Error 2
    make[2]: Target './' not remade because of errors.
    make[1]: *** [/home/sasha/build/linus-next/Makefile:1921: .] Error 2
    make[1]: Target '__all' not remade because of errors.
    make: *** [Makefile:234: __sub-make] Error 2
    make: Target '__all' not remade because of errors.

Re: [Fix CVE-2024-50217 in v6.6.y] [PATCH] btrfs: fix use-after-free of block device file in __btrfs_free_extra_devids()

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

Found matching upstream commit: aec8e6bf839101784f3ef037dcdb9432c3f32343

WARNING: Author mismatch between patch and found commit:
Backport author: Shubham Pushpkar<spushpka@cisco.com>
Commit author: Zhihao Cheng<chengzhihao1@huawei.com>


Status in newer kernel trees:
6.13.y | Present (exact SHA1)
6.12.y | Present (exact SHA1)
6.6.y | Not found

Note: The patch differs from the upstream commit:
---
1:  aec8e6bf83910 ! 1:  387b408516f7c btrfs: fix use-after-free of block device file in __btrfs_free_extra_devids()
    @@ Metadata
      ## Commit message ##
         btrfs: fix use-after-free of block device file in __btrfs_free_extra_devids()
     
    +    commit aec8e6bf839101784f3ef037dcdb9432c3f32343 ("btrfs:
    +    fix use-after-free of block device file in __btrfs_free_extra_devids()")
    +
         Mounting btrfs from two images (which have the same one fsid and two
         different dev_uuids) in certain executing order may trigger an UAF for
         variable 'device->bdev_file' in __btrfs_free_extra_devids(). And
    @@ Commit message
         Fix it by setting 'device->bdev_file' as 'NULL' after closing the
         btrfs_device in btrfs_close_one_device().
     
    +    Fixes: CVE-2024-50217
         Fixes: 142388194191 ("btrfs: do not background blkdev_put()")
         CC: stable@vger.kernel.org # 4.19+
         Link: https://bugzilla.kernel.org/show_bug.cgi?id=219408
         Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
         Reviewed-by: David Sterba <dsterba@suse.com>
         Signed-off-by: David Sterba <dsterba@suse.com>
    +    (cherry picked from commit aec8e6bf839101784f3ef037dcdb9432c3f32343)
    +    Signed-off-by: Shubham Pushpkar <spushpka@cisco.com>
     
      ## fs/btrfs/volumes.c ##
     @@ fs/btrfs/volumes.c: static void btrfs_close_one_device(struct btrfs_device *device)
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.6.y        |  Success    |  Failed    |

Build Errors:
Build error for stable/linux-6.6.y:
    lib/test_dhry.o: warning: objtool: dhry() falls through to next function dhry_run_set.cold()
    fs/btrfs/volumes.c: In function 'btrfs_close_one_device':
    fs/btrfs/volumes.c:1179:23: error: 'struct btrfs_device' has no member named 'bdev_file'
     1179 |                 device->bdev_file = NULL;
          |                       ^~
    make[4]: *** [scripts/Makefile.build:243: fs/btrfs/volumes.o] Error 1
    make[4]: Target 'fs/btrfs/' not remade because of errors.
    make[3]: *** [scripts/Makefile.build:480: fs/btrfs] Error 2
    make[3]: Target 'fs/' not remade because of errors.
    make[2]: *** [scripts/Makefile.build:480: fs] Error 2
    make[2]: Target './' not remade because of errors.
    make[1]: *** [/home/sasha/build/linus-next/Makefile:1921: .] Error 2
    make[1]: Target '__all' not remade because of errors.
    make: *** [Makefile:234: __sub-make] Error 2
    make: Target '__all' not remade because of errors.

Re: [Fix CVE-2024-50217 in v6.6.y] [PATCH] btrfs: fix use-after-free of block device file in __btrfs_free_extra_devids()

[Greg KH]

On Mon, Feb 03, 2025 at 06:45:55PM +0000, Shubham Pushpkar -X (spushpka - E INFOCHIPS PRIVATE LIMITED at Cisco) wrote:
> Thank you, Greg, for your feedback and for highlighting the importance
> of thorough testing. I apologize for any oversight in my previous
> submission. I will ensure that all future patches are rigorously
> tested before submission.

Again, they must be tested AND have a second signed-off-by from someone
else in your company when you resend them as proof of this testing by
both of you.

thanks,

greg k-h