From 1fa7603d569b9e738e9581937ba8725cd7d39b48 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 2 May 2024 22:45:58 +0200 Subject: btrfs: qgroup: update rescan message levels and error codes On filesystems without enabled quotas there's still a warning message in the logs when rescan is called. In that case it's not a problem that should be reported, rescan can be called unconditionally. Change the error code to ENOTCONN which is used for 'quotas not enabled' elsewhere. Remove message (also a warning) when rescan is called during an ongoing rescan, this brings no useful information and the error code is sufficient. Change message levels to debug for now, they can be removed eventually. CC: stable@vger.kernel.org # 6.6+ Reviewed-by: Boris Burkov Signed-off-by: David Sterba --- fs/btrfs/qgroup.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'fs') diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c index eb28141d5c37..f93354a96909 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -3820,14 +3820,14 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid, /* we're resuming qgroup rescan at mount time */ if (!(fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN)) { - btrfs_warn(fs_info, + btrfs_debug(fs_info, "qgroup rescan init failed, qgroup rescan is not queued"); ret = -EINVAL; } else if (!(fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_ON)) { - btrfs_warn(fs_info, + btrfs_debug(fs_info, "qgroup rescan init failed, qgroup is not enabled"); - ret = -EINVAL; + ret = -ENOTCONN; } if (ret) @@ -3838,14 +3838,12 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid, if (init_flags) { if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) { - btrfs_warn(fs_info, - "qgroup rescan is already in progress"); ret = -EINPROGRESS; } else if (!(fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_ON)) { - btrfs_warn(fs_info, + btrfs_debug(fs_info, "qgroup rescan init failed, qgroup is not enabled"); - ret = -EINVAL; + ret = -ENOTCONN; } else if (btrfs_qgroup_mode(fs_info) == BTRFS_QGROUP_MODE_DISABLED) { /* Quota disable is in progress */ ret = -EBUSY; -- cgit v1.2.3 From 2b8aa78cf1279ec5e418baa26bfed5df682568d8 Mon Sep 17 00:00:00 2001 From: Boris Burkov Date: Thu, 9 May 2024 15:34:40 -0700 Subject: btrfs: qgroup: fix qgroup id collision across mounts If we delete subvolumes whose ID is the largest in the filesystem, then unmount and mount again, then btrfs_init_root_free_objectid on the tree_root will select a subvolid smaller than that one and thus allow reusing it. If we are also using qgroups (and particularly squotas) it is possible to delete the subvol without deleting the qgroup. In that case, we will be able to create a new subvol whose id already has a level 0 qgroup. This will result in re-using that qgroup which would then lead to incorrect accounting. Fixes: 6ed05643ddb1 ("btrfs: create qgroup earlier in snapshot creation") CC: stable@vger.kernel.org # 6.7+ Reviewed-by: Qu Wenruo Signed-off-by: Boris Burkov Signed-off-by: David Sterba --- fs/btrfs/qgroup.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'fs') diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c index f93354a96909..fc2a7ea26354 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -468,6 +468,7 @@ int btrfs_read_qgroup_config(struct btrfs_fs_info *fs_info) } if (!qgroup) { struct btrfs_qgroup *prealloc; + struct btrfs_root *tree_root = fs_info->tree_root; prealloc = kzalloc(sizeof(*prealloc), GFP_KERNEL); if (!prealloc) { @@ -475,6 +476,25 @@ int btrfs_read_qgroup_config(struct btrfs_fs_info *fs_info) goto out; } qgroup = add_qgroup_rb(fs_info, prealloc, found_key.offset); + /* + * If a qgroup exists for a subvolume ID, it is possible + * that subvolume has been deleted, in which case + * re-using that ID would lead to incorrect accounting. + * + * Ensure that we skip any such subvol ids. + * + * We don't need to lock because this is only called + * during mount before we start doing things like creating + * subvolumes. + */ + if (is_fstree(qgroup->qgroupid) && + qgroup->qgroupid > tree_root->free_objectid) + /* + * Don't need to check against BTRFS_LAST_FREE_OBJECTID, + * as it will get checked on the next call to + * btrfs_get_free_objectid. + */ + tree_root->free_objectid = qgroup->qgroupid + 1; } ret = btrfs_sysfs_add_one_qgroup(fs_info, qgroup); if (ret < 0) -- cgit v1.2.3 From 0090d6e1b210551e63cf43958dc7a1ec942cdde9 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Wed, 8 May 2024 11:51:07 +0100 Subject: btrfs: zoned: fix use-after-free due to race with dev replace While loading a zone's info during creation of a block group, we can race with a device replace operation and then trigger a use-after-free on the device that was just replaced (source device of the replace operation). This happens because at btrfs_load_zone_info() we extract a device from the chunk map into a local variable and then use the device while not under the protection of the device replace rwsem. So if there's a device replace operation happening when we extract the device and that device is the source of the replace operation, we will trigger a use-after-free if before we finish using the device the replace operation finishes and frees the device. Fix this by enlarging the critical section under the protection of the device replace rwsem so that all uses of the device are done inside the critical section. CC: stable@vger.kernel.org # 6.1.x: 15c12fcc50a1: btrfs: zoned: introduce a zone_info struct in btrfs_load_block_group_zone_info CC: stable@vger.kernel.org # 6.1.x: 09a46725cc84: btrfs: zoned: factor out per-zone logic from btrfs_load_block_group_zone_info CC: stable@vger.kernel.org # 6.1.x: 9e0e3e74dc69: btrfs: zoned: factor out single bg handling from btrfs_load_block_group_zone_info CC: stable@vger.kernel.org # 6.1.x: 87463f7e0250: btrfs: zoned: factor out DUP bg handling from btrfs_load_block_group_zone_info CC: stable@vger.kernel.org # 6.1.x Reviewed-by: Johannes Thumshirn Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/zoned.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'fs') diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c index 4cba80b34387..459514da1660 100644 --- a/fs/btrfs/zoned.c +++ b/fs/btrfs/zoned.c @@ -1290,7 +1290,7 @@ static int btrfs_load_zone_info(struct btrfs_fs_info *fs_info, int zone_idx, struct btrfs_chunk_map *map) { struct btrfs_dev_replace *dev_replace = &fs_info->dev_replace; - struct btrfs_device *device = map->stripes[zone_idx].dev; + struct btrfs_device *device; int dev_replace_is_ongoing = 0; unsigned int nofs_flag; struct blk_zone zone; @@ -1298,7 +1298,11 @@ static int btrfs_load_zone_info(struct btrfs_fs_info *fs_info, int zone_idx, info->physical = map->stripes[zone_idx].physical; + down_read(&dev_replace->rwsem); + device = map->stripes[zone_idx].dev; + if (!device->bdev) { + up_read(&dev_replace->rwsem); info->alloc_offset = WP_MISSING_DEV; return 0; } @@ -1308,6 +1312,7 @@ static int btrfs_load_zone_info(struct btrfs_fs_info *fs_info, int zone_idx, __set_bit(zone_idx, active); if (!btrfs_dev_is_sequential(device, info->physical)) { + up_read(&dev_replace->rwsem); info->alloc_offset = WP_CONVENTIONAL; return 0; } @@ -1315,11 +1320,9 @@ static int btrfs_load_zone_info(struct btrfs_fs_info *fs_info, int zone_idx, /* This zone will be used for allocation, so mark this zone non-empty. */ btrfs_dev_clear_zone_empty(device, info->physical); - down_read(&dev_replace->rwsem); dev_replace_is_ongoing = btrfs_dev_replace_is_ongoing(dev_replace); if (dev_replace_is_ongoing && dev_replace->tgtdev != NULL) btrfs_dev_clear_zone_empty(dev_replace->tgtdev, info->physical); - up_read(&dev_replace->rwsem); /* * The group is mapped to a sequential zone. Get the zone write pointer @@ -1330,6 +1333,7 @@ static int btrfs_load_zone_info(struct btrfs_fs_info *fs_info, int zone_idx, ret = btrfs_get_dev_zone(device, info->physical, &zone); memalloc_nofs_restore(nofs_flag); if (ret) { + up_read(&dev_replace->rwsem); if (ret != -EIO && ret != -EOPNOTSUPP) return ret; info->alloc_offset = WP_MISSING_DEV; @@ -1341,6 +1345,7 @@ static int btrfs_load_zone_info(struct btrfs_fs_info *fs_info, int zone_idx, "zoned: unexpected conventional zone %llu on device %s (devid %llu)", zone.start << SECTOR_SHIFT, rcu_str_deref(device->name), device->devid); + up_read(&dev_replace->rwsem); return -EIO; } @@ -1368,6 +1373,8 @@ static int btrfs_load_zone_info(struct btrfs_fs_info *fs_info, int zone_idx, break; } + up_read(&dev_replace->rwsem); + return 0; } -- cgit v1.2.3 From b4e585fffc1cf877112ed231a91f089e85688c2a Mon Sep 17 00:00:00 2001 From: Lu Yao Date: Tue, 7 May 2024 10:34:17 +0800 Subject: btrfs: scrub: initialize ret in scrub_simple_mirror() to fix compilation warning The following error message is displayed: ../fs/btrfs/scrub.c:2152:9: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]" Compiler version: gcc version: (Debian 10.2.1-6) 10.2.1 20210110 Reviewed-by: Boris Burkov Signed-off-by: Lu Yao Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/scrub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c index 4b22cfe9a98c..afd6932f5e89 100644 --- a/fs/btrfs/scrub.c +++ b/fs/btrfs/scrub.c @@ -2100,7 +2100,7 @@ static int scrub_simple_mirror(struct scrub_ctx *sctx, struct btrfs_fs_info *fs_info = sctx->fs_info; const u64 logical_end = logical_start + logical_length; u64 cur_logical = logical_start; - int ret; + int ret = 0; /* The range must be inside the bg */ ASSERT(logical_start >= bg->start && logical_end <= bg->start + bg->length); -- cgit v1.2.3 From dddff821b660e08eb8418cfd3909e61204804534 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Tue, 14 May 2024 15:36:35 +0100 Subject: btrfs: fix end of tree detection when searching for data extent ref At lookup_extent_data_ref() we are incorrectly checking if we are at the last slot of the last leaf in the extent tree. We are returning -ENOENT if btrfs_next_leaf() returns a value greater than 1, but btrfs_next_leaf() never returns anything greater than 1: 1) It returns < 0 on error; 2) 0 if there is a next leaf (or a new item was added to the end of the current leaf after releasing the path); 3) 1 if there are no more leaves (and no new items were added to the last leaf after releasing the path). So fix this by checking if the return value is greater than zero instead of being greater than one. Fixes: 1618aa3c2e01 ("btrfs: simplify return variables in lookup_extent_data_ref()") Reviewed-by: Qu Wenruo Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/extent-tree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 47d48233b592..3774c191e36d 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -477,7 +477,7 @@ again: if (path->slots[0] >= nritems) { ret = btrfs_next_leaf(root, path); if (ret) { - if (ret > 1) + if (ret > 0) return -ENOENT; return ret; } -- cgit v1.2.3 From 440861b1a03c72cc7be4a307e178dcaa6894479b Mon Sep 17 00:00:00 2001 From: Qu Wenruo Date: Tue, 21 May 2024 19:27:31 +0930 Subject: btrfs: re-introduce 'norecovery' mount option Although 'norecovery' mount option was marked as deprecated for a long time and a warning message was printed during the deprecation window, it's still actively utilized by several projects that need a safer way to mount a btrfs without any writes. Furthermore this 'norecovery' mount option is supported by other major filesystems, which makes it less clear what's our motivation to remove it. Re-introduce the 'norecovery' mount option, and output a message to recommend 'rescue=nologreplay' option. Link: https://lore.kernel.org/linux-btrfs/ZkxZT0J-z0GYvfy8@gardel-login/#t Link: https://github.com/systemd/systemd/pull/32892 Link: https://bugzilla.suse.com/show_bug.cgi?id=1222429 Reported-by: Lennart Poettering Reported-by: Jiri Slaby Fixes: a1912f712188 ("btrfs: remove code for inode_cache and recovery mount options") CC: stable@vger.kernel.org # 6.8+ Reviewed-by: Johannes Thumshirn Signed-off-by: Qu Wenruo Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/super.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'fs') diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 2dbc930a20f7..f05cce7c8b8d 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -119,6 +119,7 @@ enum { Opt_thread_pool, Opt_treelog, Opt_user_subvol_rm_allowed, + Opt_norecovery, /* Rescue options */ Opt_rescue, @@ -245,6 +246,8 @@ static const struct fs_parameter_spec btrfs_fs_parameters[] = { __fsparam(NULL, "nologreplay", Opt_nologreplay, fs_param_deprecated, NULL), /* Deprecated, with alias rescue=usebackuproot */ __fsparam(NULL, "usebackuproot", Opt_usebackuproot, fs_param_deprecated, NULL), + /* For compatibility only, alias for "rescue=nologreplay". */ + fsparam_flag("norecovery", Opt_norecovery), /* Debugging options. */ fsparam_flag_no("enospc_debug", Opt_enospc_debug), @@ -438,6 +441,11 @@ static int btrfs_parse_param(struct fs_context *fc, struct fs_parameter *param) "'nologreplay' is deprecated, use 'rescue=nologreplay' instead"); btrfs_set_opt(ctx->mount_opt, NOLOGREPLAY); break; + case Opt_norecovery: + btrfs_info(NULL, +"'norecovery' is for compatibility only, recommended to use 'rescue=nologreplay'"); + btrfs_set_opt(ctx->mount_opt, NOLOGREPLAY); + break; case Opt_flushoncommit: if (result.negated) btrfs_clear_opt(ctx->mount_opt, FLUSHONCOMMIT); -- cgit v1.2.3