btrfs-progs: mkfs: fix a stack over-flow when features string are too long

[BUG]
Even with chunk_objectid bug fixed, mkfs.btrfs can still caused stack
overflow when enabling extent-tree-v2 feature (need experimental
features enabled):

  # ./mkfs.btrfs  -f -O extent-tree-v2 ~/test.img
  btrfs-progs v5.19.1
  See http://btrfs.wiki.kernel.org for more information.

  ERROR: superblock magic doesn't match
  NOTE: several default settings have changed in version 5.15, please make sure
        this does not affect your deployments:
        - DUP for metadata (-m dup)
        - enabled no-holes (-O no-holes)
        - enabled free-space-tree (-R free-space-tree)

  Label:              (null)
  UUID:               205c61e7-f58e-4e8f-9dc2-38724f5c554b
  Node size:          16384
  Sector size:        4096
  Filesystem size:    512.00MiB
  Block group profiles:
    Data:             single            8.00MiB
    Metadata:         DUP              32.00MiB
    System:           DUP               8.00MiB
  SSD detected:       no
  Zoned device:       no
  =================================================================
  [... Skip full ASAN output ...]
  ==65655==ABORTING

[CAUSE]
For experimental build, we have unified feature output, but the old
buffer size is only 64 bytes, which is too small to cover the new full
feature string:

  extref, skinny-metadata, no-holes, free-space-tree, block-group-tree, extent-tree-v2

Above feature string is already 84 bytes, over the 64 on-stack memory
size.

This can also be proved by the ASAN output:

  ==65655==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffc4e03b1d0 at pc 0x7ff0fc05fafe bp 0x7ffc4e03ac60 sp 0x7ffc4e03a408
  WRITE of size 17 at 0x7ffc4e03b1d0 thread T0
      #0 0x7ff0fc05fafd in __interceptor_strcat /usr/src/debug/gcc/libsanitizer/asan/asan_interceptors.cpp:377
      #1 0x55cdb7b06ca5 in parse_features_to_string common/fsfeatures.c:316
      #2 0x55cdb7b06ce1 in btrfs_parse_fs_features_to_string common/fsfeatures.c:324
      #3 0x55cdb7a37226 in main mkfs/main.c:1783
      #4 0x7ff0fbe3c28f  (/usr/lib/libc.so.6+0x2328f)
      #5 0x7ff0fbe3c349 in __libc_start_main (/usr/lib/libc.so.6+0x23349)
      #6 0x55cdb7a2cb34 in _start ../sysdeps/x86_64/start.S:115

[FIX]
Introduce a new macro, BTRFS_FEATURE_STRING_BUF_SIZE, along with a new
sanity check helper, btrfs_assert_feature_buf_size().

The problem is I can not find a build time method to verify
BTRFS_FEATURE_STRING_BUF_SIZE is large enough to contain all feature
names, thus have to go the runtime function to do the BUG_ON() to verify
the macro size.

Now the minimal buffer size for experimental build is 138 bytes, just
bump it to 160 for future expansion.

And if further features go beyond that number, mkfs.btrfs/btrfs-convert
will immediately crash at that BUG_ON(), so we can definitely detect it.

Reviewed-by: Anand Jain <anand.jain@oracle.com>
Tested-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
This commit is contained in:
Qu Wenruo 2022-10-07 20:03:01 +08:00 committed by David Sterba
parent 56e75c9f75
commit d8f1bd519f
4 changed files with 37 additions and 2 deletions

View File

@ -251,6 +251,32 @@ static const struct btrfs_feature runtime_features[] = {
}
};
/*
* This is a sanity check to make sure BTRFS_FEATURE_STRING_BUF_SIZE is large
* enough to contain all strings.
*
* All callers using btrfs_parse_*_features_to_string() should call this first.
*/
void btrfs_assert_feature_buf_size(void)
{
int total_size = 0;
int i;
/*
* This is a little over-calculated, as we include ", list-all".
* But 10 extra bytes should not be a big deal.
*/
for (i = 0; i < ARRAY_SIZE(mkfs_features); i++)
/* The extra 2 bytes are for the ", " prefix. */
total_size += strlen(mkfs_features[i].name) + 2;
BUG_ON(BTRFS_FEATURE_STRING_BUF_SIZE < total_size);
total_size = 0;
for (i = 0; i < ARRAY_SIZE(runtime_features); i++)
total_size += strlen(runtime_features[i].name) + 2;
BUG_ON(BTRFS_FEATURE_STRING_BUF_SIZE < total_size);
}
static size_t get_feature_array_size(enum feature_source source)
{
if (source == FS_FEATURES)

View File

@ -37,6 +37,12 @@ struct btrfs_mkfs_features {
#define BTRFS_FEATURE_RUNTIME_QUOTA (1ULL << 0)
#define BTRFS_FEATURE_RUNTIME_LIST_ALL (1ULL << 1)
/*
* Such buffer size should be able to contain all feature string, with extra
* ", " for each feature.
*/
#define BTRFS_FEATURE_STRING_BUF_SIZE (160)
static const struct btrfs_mkfs_features btrfs_mkfs_default_features = {
.compat_ro_flags = BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE |
BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID,
@ -86,5 +92,6 @@ int btrfs_check_sectorsize(u32 sectorsize);
int btrfs_check_features(const struct btrfs_mkfs_features *features,
const struct btrfs_mkfs_features *allowed);
int btrfs_tree_search2_ioctl_supported(int fd);
void btrfs_assert_feature_buf_size(void);
#endif

View File

@ -1147,7 +1147,7 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize,
struct btrfs_key key;
char subvol_name[SOURCE_FS_NAME_LEN + 8];
struct task_ctx ctx;
char features_buf[64];
char features_buf[BTRFS_FEATURE_STRING_BUF_SIZE];
char fsid_str[BTRFS_UUID_UNPARSED_SIZE];
struct btrfs_mkfs_config mkfs_cfg;
bool btrfs_sb_committed = false;
@ -1835,6 +1835,7 @@ int BOX_MAIN(convert)(int argc, char *argv[])
char fsid[BTRFS_UUID_UNPARSED_SIZE] = {0};
crc32c_optimization_init();
btrfs_assert_feature_buf_size();
printf("btrfs-convert from %s\n\n", PACKAGE_STRING);
while(1) {

View File

@ -1028,6 +1028,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
crc32c_optimization_init();
btrfs_config_init();
btrfs_assert_feature_buf_size();
while(1) {
int c;
@ -1750,7 +1751,7 @@ raid_groups:
}
}
if (bconf.verbose) {
char features_buf[64];
char features_buf[BTRFS_FEATURE_STRING_BUF_SIZE];
update_chunk_allocation(fs_info, &allocation);
printf("Label: %s\n", label);