From 4db925911c6bbbe55fee0af4287a39685094c9ed Mon Sep 17 00:00:00 2001 From: David Sterba Date: Mon, 17 Jun 2024 20:56:38 +0200 Subject: [PATCH] btrfs-progs: use strncpy_null everywhere MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the safe version of strncpy that makes sure the string is terminated. To be noted: - the conversion in scrub path handling was skipped - sizes of device paths in some ioctl related structures is BTRFS_DEVICE_PATH_NAME_MAX + 1 Recently gcc 13.3 started to detect problems with our use of strncpy potentially lacking the null terminator, warnings like: cmds/inspect.c: In function ‘cmd_inspect_logical_resolve’: cmds/inspect.c:294:33: warning: ‘__builtin_strncpy’ specified bound 4096 equals destination size [-Wstringop-truncation] 294 | strncpy(mount_path, mounted, PATH_MAX); | ^ Signed-off-by: David Sterba --- btrfs-corrupt-block.c | 2 +- cmds/device.c | 5 ++--- cmds/inspect.c | 4 ++-- cmds/replace.c | 8 ++++---- cmds/restore.c | 3 +-- cmds/scrub.c | 2 +- cmds/subvolume-list.c | 6 ++---- common/help.c | 3 +-- common/open-utils.c | 8 ++++---- convert/main.c | 2 +- convert/source-ext2.c | 3 ++- kernel-shared/print-tree.c | 9 +++++---- mkfs/main.c | 5 ++--- 13 files changed, 28 insertions(+), 32 deletions(-) diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c index e8831989..fc71eb2c 100644 --- a/btrfs-corrupt-block.c +++ b/btrfs-corrupt-block.c @@ -1398,7 +1398,7 @@ int main(int argc, char **argv) inode = arg_strtou64(optarg); break; case 'f': - strncpy(field, optarg, FIELD_BUF_LEN); + strncpy_null(field, optarg, FIELD_BUF_LEN); break; case 'x': file_extent = arg_strtou64(optarg); diff --git a/cmds/device.c b/cmds/device.c index 6a9a4db2..2a08dd5c 100644 --- a/cmds/device.c +++ b/cmds/device.c @@ -799,9 +799,8 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv) char path[BTRFS_DEVICE_PATH_NAME_MAX + 1]; int err2; - strncpy(path, (char *)di_args[i].path, - BTRFS_DEVICE_PATH_NAME_MAX); - path[BTRFS_DEVICE_PATH_NAME_MAX] = 0; + strncpy_null(path, (char *)di_args[i].path, + BTRFS_DEVICE_PATH_NAME_MAX + 1); args.devid = di_args[i].devid; args.nr_items = BTRFS_DEV_STAT_VALUES_MAX; diff --git a/cmds/inspect.c b/cmds/inspect.c index 5b3f3c05..353ed639 100644 --- a/cmds/inspect.c +++ b/cmds/inspect.c @@ -258,7 +258,7 @@ static int cmd_inspect_logical_resolve(const struct cmd_struct *cmd, if (name[0] == 0) { path_ptr[-1] = '\0'; path_fd = fd; - strncpy(mount_path, full_path, PATH_MAX); + strncpy_null(mount_path, full_path, PATH_MAX); } else { char *mounted = NULL; char subvol[PATH_MAX]; @@ -291,7 +291,7 @@ static int cmd_inspect_logical_resolve(const struct cmd_struct *cmd, continue; } - strncpy(mount_path, mounted, PATH_MAX); + strncpy_null(mount_path, mounted, PATH_MAX); free(mounted); path_fd = btrfs_open_dir(mount_path); diff --git a/cmds/replace.c b/cmds/replace.c index c1eec84c..5f1222b2 100644 --- a/cmds/replace.c +++ b/cmds/replace.c @@ -266,8 +266,8 @@ static int cmd_replace_start(const struct cmd_struct *cmd, goto leave_with_error; } } else if (path_is_block_device(srcdev) > 0) { - strncpy((char *)start_args.start.srcdev_name, srcdev, - BTRFS_DEVICE_PATH_NAME_MAX); + strncpy_null((char *)start_args.start.srcdev_name, srcdev, + BTRFS_DEVICE_PATH_NAME_MAX + 1); start_args.start.srcdevid = 0; srcdev_size = device_get_partition_size(srcdev); } else { @@ -292,8 +292,8 @@ static int cmd_replace_start(const struct cmd_struct *cmd, goto leave_with_error; } - strncpy((char *)start_args.start.tgtdev_name, dstdev, - BTRFS_DEVICE_PATH_NAME_MAX); + strncpy_null((char *)start_args.start.tgtdev_name, dstdev, + BTRFS_DEVICE_PATH_NAME_MAX + 1); ret = btrfs_prepare_device(fddstdev, dstdev, &dstdev_block_count, 0, PREP_DEVICE_ZERO_END | PREP_DEVICE_VERBOSE | (discard ? PREP_DEVICE_DISCARD : 0) | diff --git a/cmds/restore.c b/cmds/restore.c index ca2a13a1..6bc619b3 100644 --- a/cmds/restore.c +++ b/cmds/restore.c @@ -1535,8 +1535,7 @@ static int cmd_restore(const struct cmd_struct *cmd, int argc, char **argv) ret = 1; goto out; } - strncpy(dir_name, argv[optind + 1], sizeof dir_name); - dir_name[sizeof dir_name - 1] = 0; + strncpy_null(dir_name, argv[optind + 1], sizeof(dir_name)); /* Strip the trailing / on the dir name */ len = strlen(dir_name); diff --git a/cmds/scrub.c b/cmds/scrub.c index 03c1f7b1..202690db 100644 --- a/cmds/scrub.c +++ b/cmds/scrub.c @@ -1437,7 +1437,7 @@ static int scrub_start(const struct cmd_struct *cmd, int argc, char **argv, sock_path, sizeof(sock_path)); /* ignore EOVERFLOW, try using a shorter path for the socket */ addr.sun_path[sizeof(addr.sun_path) - 1] = '\0'; - strncpy(addr.sun_path, sock_path, sizeof(addr.sun_path) - 1); + strncpy_null(addr.sun_path, sock_path, sizeof(addr.sun_path)); ret = bind(prg_fd, (struct sockaddr *)&addr, sizeof(addr)); if (ret != -1 || errno != EADDRINUSE) break; diff --git a/cmds/subvolume-list.c b/cmds/subvolume-list.c index 21319267..52a30aa0 100644 --- a/cmds/subvolume-list.c +++ b/cmds/subvolume-list.c @@ -547,8 +547,7 @@ static int update_root(struct rb_root *root_lookup, error_msg(ERROR_MSG_MEMORY, NULL); exit(1); } - strncpy(ri->name, name, name_len); - ri->name[name_len] = 0; + strncpy_null(ri->name, name, name_len); } if (ref_tree) ri->ref_tree = ref_tree; @@ -619,8 +618,7 @@ static int add_root(struct rb_root *root_lookup, error_msg(ERROR_MSG_MEMORY, NULL); exit(1); } - strncpy(ri->name, name, name_len); - ri->name[name_len] = 0; + strncpy_null(ri->name, name, name_len); } if (ref_tree) ri->ref_tree = ref_tree; diff --git a/common/help.c b/common/help.c index c302d375..6cf8e2a9 100644 --- a/common/help.c +++ b/common/help.c @@ -47,8 +47,7 @@ void fixup_argv0(char **argv, const char *token) void set_argv0(char **argv) { - strncpy(argv0_buf, argv[0], sizeof(argv0_buf)); - argv0_buf[sizeof(argv0_buf) - 1] = 0; + strncpy_null(argv0_buf, argv[0], sizeof(argv0_buf)); } int check_argc_exact(int nargs, int expected) diff --git a/common/open-utils.c b/common/open-utils.c index 95294f63..8490be4a 100644 --- a/common/open-utils.c +++ b/common/open-utils.c @@ -32,6 +32,7 @@ #include "common/messages.h" #include "common/path-utils.h" #include "common/device-scan.h" +#include "common/string-utils.h" #include "common/open-utils.h" /* @@ -103,10 +104,9 @@ int check_mounted_where(int fd, const char *file, char *where, int size, } /* Did we find an entry in mnt table? */ - if (mnt && size && where) { - strncpy(where, mnt->mnt_dir, size); - where[size-1] = 0; - } + if (mnt && size && where) + strncpy_null(where, mnt->mnt_dir, size); + if (fs_dev_ret) *fs_dev_ret = fs_devices_mnt; else if (noscan) diff --git a/convert/main.c b/convert/main.c index 48bcff2f..8e73aa25 100644 --- a/convert/main.c +++ b/convert/main.c @@ -1996,7 +1996,7 @@ int BOX_MAIN(convert)(int argc, char *argv[]) error("invalid UUID: %s\n", optarg); return 1; } - strncpy(fsid, optarg, sizeof(fsid)); + strncpy_null(fsid, optarg, sizeof(fsid)); } break; case GETOPT_VAL_HELP: diff --git a/convert/source-ext2.c b/convert/source-ext2.c index 00a7b290..5efaaa4b 100644 --- a/convert/source-ext2.c +++ b/convert/source-ext2.c @@ -30,6 +30,7 @@ #include "kernel-shared/file-item.h" #include "common/extent-cache.h" #include "common/messages.h" +#include "common/string-utils.h" #include "convert/common.h" #include "convert/source-fs.h" #include "convert/source-ext2.h" @@ -638,7 +639,7 @@ static int ext2_copy_single_xattr(struct btrfs_trans_handle *trans, data = databuf; datalen = bufsize; } - strncpy(namebuf, xattr_prefix_table[name_index], XATTR_NAME_MAX); + strncpy_null(namebuf, xattr_prefix_table[name_index], XATTR_NAME_MAX); strncat(namebuf, EXT2_EXT_ATTR_NAME(entry), entry->e_name_len); if (name_len + datalen > BTRFS_LEAF_DATA_SIZE(root->fs_info) - sizeof(struct btrfs_item) - sizeof(struct btrfs_dir_item)) { diff --git a/kernel-shared/print-tree.c b/kernel-shared/print-tree.c index 568a84cb..6f78ec35 100644 --- a/kernel-shared/print-tree.c +++ b/kernel-shared/print-tree.c @@ -36,6 +36,7 @@ #include "common/defs.h" #include "common/internal.h" #include "common/messages.h" +#include "common/string-utils.h" #include "uapi/btrfs.h" static void print_dir_item_type(struct extent_buffer *eb, @@ -186,7 +187,7 @@ static void bg_flags_to_str(u64 flags, char *ret) ret[0] = '\0'; if (flags & BTRFS_BLOCK_GROUP_DATA) { empty = 0; - strncpy(ret, "DATA", BG_FLAG_STRING_LEN); + strncpy_null(ret, "DATA", BG_FLAG_STRING_LEN); } if (flags & BTRFS_BLOCK_GROUP_METADATA) { if (!empty) @@ -209,7 +210,7 @@ static void bg_flags_to_str(u64 flags, char *ret) * Thus here we only fill @profile if it's not single. */ if (strncmp(name, "SINGLE", strlen("SINGLE")) != 0) - strncpy(profile, name, BG_FLAG_STRING_LEN); + strncpy_null(profile, name, BG_FLAG_STRING_LEN); } if (profile[0]) { strncat(ret, "|", BG_FLAG_STRING_LEN); @@ -1398,7 +1399,7 @@ static void print_header_info(struct extent_buffer *eb, unsigned int mode) #define DEV_REPLACE_STRING_LEN 64 #define CASE_DEV_REPLACE_MODE_ENTRY(dest, name) \ case BTRFS_DEV_REPLACE_ITEM_CONT_READING_FROM_SRCDEV_MODE_##name: \ - strncpy((dest), #name, DEV_REPLACE_STRING_LEN); \ + strncpy_null((dest), #name, DEV_REPLACE_STRING_LEN); \ break; static void replace_mode_to_str(u64 flags, char *ret) @@ -1415,7 +1416,7 @@ static void replace_mode_to_str(u64 flags, char *ret) #define CASE_DEV_REPLACE_STATE_ENTRY(dest, name) \ case BTRFS_IOCTL_DEV_REPLACE_STATE_##name: \ - strncpy((dest), #name, DEV_REPLACE_STRING_LEN); \ + strncpy_null((dest), #name, DEV_REPLACE_STRING_LEN); \ break; static void replace_state_to_str(u64 flags, char *ret) diff --git a/mkfs/main.c b/mkfs/main.c index 8d0924ea..b40f7432 100644 --- a/mkfs/main.c +++ b/mkfs/main.c @@ -1361,8 +1361,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv) source_dir = strdup(optarg); break; case 'U': - strncpy(fs_uuid, optarg, - BTRFS_UUID_UNPARSED_SIZE - 1); + strncpy_null(fs_uuid, optarg, BTRFS_UUID_UNPARSED_SIZE); break; case 'K': opt_discard = false; @@ -1371,7 +1370,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv) bconf_be_quiet(); break; case GETOPT_VAL_DEVICE_UUID: - strncpy(dev_uuid, optarg, BTRFS_UUID_UNPARSED_SIZE - 1); + strncpy_null(dev_uuid, optarg, BTRFS_UUID_UNPARSED_SIZE); break; case GETOPT_VAL_SHRINK: shrink_rootdir = true;