From 04837c8dcbb3c6d5965a14429b093dd0bcf214e1 Mon Sep 17 00:00:00 2001 From: Jorgen Lundman Date: Mon, 8 Jun 2020 03:42:12 +0900 Subject: [PATCH] Replace sprintf()->snprintf() and strcpy()->strlcpy() The strcpy() and sprintf() functions are deprecated on some platforms. Care is needed to ensure correct size is used. If some platforms miss snprintf, we can add a #define to sprintf, likewise strlcpy(). The biggest change is adding a size parameter to zfs_id_to_fuidstr(). The various *_impl_get() functions are only used on linux and have not yet been updated. --- module/icp/algs/aes/aes_impl.c | 4 ++-- module/icp/algs/modes/gcm.c | 4 ++-- module/icp/os/modhash.c | 6 ++++-- module/lua/lstrlib.c | 14 +++++++------- module/zfs/ddt.c | 2 +- module/zfs/dmu_objset.c | 11 ++++++----- module/zfs/dsl_dataset.c | 14 +++++++++----- module/zfs/dsl_dir.c | 5 +++-- module/zfs/dsl_prop.c | 10 ++++++---- module/zfs/dsl_scan.c | 2 +- module/zfs/dsl_userhold.c | 6 +++--- module/zfs/spa.c | 4 ++-- module/zfs/zap.c | 2 +- module/zfs/zap_micro.c | 3 ++- module/zfs/zcp_get.c | 18 ++++++++++-------- module/zfs/zfs_ioctl.c | 5 +++-- module/zfs/zio.c | 6 ++++-- 17 files changed, 66 insertions(+), 50 deletions(-) diff --git a/module/icp/algs/aes/aes_impl.c b/module/icp/algs/aes/aes_impl.c index fe15d76d1..571fdd353 100644 --- a/module/icp/algs/aes/aes_impl.c +++ b/module/icp/algs/aes/aes_impl.c @@ -330,7 +330,7 @@ aes_impl_init(void) sizeof (aes_fastest_impl)); #endif - strcpy(aes_fastest_impl.name, "fastest"); + strlcpy(aes_fastest_impl.name, "fastest", AES_IMPL_NAME_MAX); /* Finish initialization */ atomic_swap_32(&icp_aes_impl, user_sel_impl); @@ -405,7 +405,7 @@ aes_impl_set(const char *val) return (err); } -#if defined(_KERNEL) +#if defined(_KERNEL) && defined(__linux__) #include static int diff --git a/module/icp/algs/modes/gcm.c b/module/icp/algs/modes/gcm.c index 0b89b0ef4..323c0dd10 100644 --- a/module/icp/algs/modes/gcm.c +++ b/module/icp/algs/modes/gcm.c @@ -857,7 +857,7 @@ gcm_impl_init(void) sizeof (gcm_fastest_impl)); } - strcpy(gcm_fastest_impl.name, "fastest"); + strlcpy(gcm_fastest_impl.name, "fastest", GCM_IMPL_NAME_MAX); #ifdef CAN_USE_GCM_ASM /* @@ -969,7 +969,7 @@ gcm_impl_set(const char *val) return (err); } -#if defined(_KERNEL) +#if defined(_KERNEL) && defined(__linux__) #include static int diff --git a/module/icp/os/modhash.c b/module/icp/os/modhash.c index 5e216ed6a..a89787100 100644 --- a/module/icp/os/modhash.c +++ b/module/icp/os/modhash.c @@ -453,17 +453,19 @@ mod_hash_create_extended( int sleep) /* whether to sleep for mem */ { mod_hash_t *mod_hash; + size_t size; ASSERT(hname && keycmp && hash_alg && vdtor && kdtor); if ((mod_hash = kmem_zalloc(MH_SIZE(nchains), sleep)) == NULL) return (NULL); - mod_hash->mh_name = kmem_alloc(strlen(hname) + 1, sleep); + size = strlen(hname) + 1; + mod_hash->mh_name = kmem_alloc(size, sleep); if (mod_hash->mh_name == NULL) { kmem_free(mod_hash, MH_SIZE(nchains)); return (NULL); } - (void) strcpy(mod_hash->mh_name, hname); + (void) strlcpy(mod_hash->mh_name, hname, size); rw_init(&mod_hash->mh_contents, NULL, RW_DEFAULT, NULL); mod_hash->mh_sleep = sleep; diff --git a/module/lua/lstrlib.c b/module/lua/lstrlib.c index 49ba70faf..12027757b 100644 --- a/module/lua/lstrlib.c +++ b/module/lua/lstrlib.c @@ -853,9 +853,9 @@ static void addquoted (lua_State *L, luaL_Buffer *b, int arg) { else if (*s == '\0' || iscntrl(uchar(*s))) { char buff[10]; if (!isdigit(uchar(*(s+1)))) - sprintf(buff, "\\%d", (int)uchar(*s)); + snprintf(buff, sizeof(buff), "\\%d", (int)uchar(*s)); else - sprintf(buff, "\\%03d", (int)uchar(*s)); + snprintf(buff, sizeof(buff), "\\%03d", (int)uchar(*s)); luaL_addstring(b, buff); } else @@ -890,11 +890,11 @@ static const char *scanformat (lua_State *L, const char *strfrmt, char *form) { /* ** add length modifier into formats */ -static void addlenmod (char *form, const char *lenmod) { +static void addlenmod (char *form, const char *lenmod, size_t size) { size_t l = strlen(form); size_t lm = strlen(lenmod); char spec = form[l - 1]; - strcpy(form + l - 1, lenmod); + strlcpy(form + l - 1, lenmod, size - (l - 1)); form[l + lm - 1] = spec; form[l + lm] = '\0'; } @@ -931,7 +931,7 @@ static int str_format (lua_State *L) { lua_Number diff = n - (lua_Number)ni; luaL_argcheck(L, -1 < diff && diff < 1, arg, "not a number in proper range"); - addlenmod(form, LUA_INTFRMLEN); + addlenmod(form, LUA_INTFRMLEN, MAX_FORMAT); nb = str_sprintf(buff, form, ni); break; } @@ -941,7 +941,7 @@ static int str_format (lua_State *L) { lua_Number diff = n - (lua_Number)ni; luaL_argcheck(L, -1 < diff && diff < 1, arg, "not a non-negative number in proper range"); - addlenmod(form, LUA_INTFRMLEN); + addlenmod(form, LUA_INTFRMLEN, MAX_FORMAT); nb = str_sprintf(buff, form, ni); break; } @@ -951,7 +951,7 @@ static int str_format (lua_State *L) { case 'a': case 'A': #endif case 'g': case 'G': { - addlenmod(form, LUA_FLTFRMLEN); + addlenmod(form, LUA_FLTFRMLEN, MAX_FORMAT); nb = str_sprintf(buff, form, (LUA_FLTFRM_T)luaL_checknumber(L, arg)); break; } diff --git a/module/zfs/ddt.c b/module/zfs/ddt.c index a38c2b24e..9be17e533 100644 --- a/module/zfs/ddt.c +++ b/module/zfs/ddt.c @@ -253,7 +253,7 @@ void ddt_object_name(ddt_t *ddt, enum ddt_type type, enum ddt_class class, char *name) { - (void) sprintf(name, DMU_POOL_DDT, + (void) snprintf(name, DDT_NAMELEN, DMU_POOL_DDT, zio_checksum_table[ddt->ddt_checksum].ci_name, ddt_ops[type]->ddt_op_name, ddt_class_name[class]); } diff --git a/module/zfs/dmu_objset.c b/module/zfs/dmu_objset.c index c78019d05..4d3c2385b 100644 --- a/module/zfs/dmu_objset.c +++ b/module/zfs/dmu_objset.c @@ -1967,14 +1967,15 @@ do_userquota_update(objset_t *os, userquota_cache_t *cache, uint64_t used, if (subtract) delta = -delta; - (void) sprintf(name, "%llx", (longlong_t)user); + (void) snprintf(name, sizeof (name), "%llx", (longlong_t)user); userquota_update_cache(&cache->uqc_user_deltas, name, delta); - (void) sprintf(name, "%llx", (longlong_t)group); + (void) snprintf(name, sizeof (name), "%llx", (longlong_t)group); userquota_update_cache(&cache->uqc_group_deltas, name, delta); if (dmu_objset_projectquota_enabled(os)) { - (void) sprintf(name, "%llx", (longlong_t)project); + (void) snprintf(name, sizeof (name), "%llx", + (longlong_t)project); userquota_update_cache(&cache->uqc_project_deltas, name, delta); } @@ -2537,7 +2538,7 @@ dmu_snapshot_list_next(objset_t *os, int namelen, char *name, return (SET_ERROR(ENAMETOOLONG)); } - (void) strcpy(name, attr.za_name); + (void) strlcpy(name, attr.za_name, namelen); if (idp) *idp = attr.za_first_integer; if (case_conflict) @@ -2582,7 +2583,7 @@ dmu_dir_list_next(objset_t *os, int namelen, char *name, return (SET_ERROR(ENAMETOOLONG)); } - (void) strcpy(name, attr.za_name); + (void) strlcpy(name, attr.za_name, namelen); if (idp) *idp = attr.za_first_integer; zap_cursor_advance(&cursor); diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c index 33b8cafbf..9de51db16 100644 --- a/module/zfs/dsl_dataset.c +++ b/module/zfs/dsl_dataset.c @@ -846,7 +846,7 @@ void dsl_dataset_name(dsl_dataset_t *ds, char *name) { if (ds == NULL) { - (void) strcpy(name, "mos"); + (void) strlcpy(name, "mos", ZFS_MAX_DATASET_NAME_LEN); } else { dsl_dir_name(ds->ds_dir, name); VERIFY0(dsl_dataset_get_snapname(ds)); @@ -2161,9 +2161,12 @@ get_receive_resume_stats_impl(dsl_dataset_t *ds) zio_cksum_t cksum; fletcher_4_native_varsize(compressed, compressed_size, &cksum); - str = kmem_alloc(compressed_size * 2 + 1, KM_SLEEP); + size_t alloc_size = compressed_size * 2 + 1; + str = kmem_alloc(alloc_size, KM_SLEEP); for (int i = 0; i < compressed_size; i++) { - (void) sprintf(str + i * 2, "%02x", compressed[i]); + size_t offset = i * 2; + (void) snprintf(str + offset, alloc_size - offset, + "%02x", compressed[i]); } str[compressed_size * 2] = '\0'; char *propval = kmem_asprintf("%u-%llx-%llx-%s", @@ -2171,7 +2174,7 @@ get_receive_resume_stats_impl(dsl_dataset_t *ds) (longlong_t)cksum.zc_word[0], (longlong_t)packed_size, str); kmem_free(packed, packed_size); - kmem_free(str, compressed_size * 2 + 1); + kmem_free(str, alloc_size); kmem_free(compressed, packed_size); return (propval); } @@ -3573,7 +3576,8 @@ dsl_dataset_promote(const char *name, char *conflsnap) */ snap_pair = nvlist_next_nvpair(ddpa.err_ds, NULL); if (snap_pair != NULL && conflsnap != NULL) - (void) strcpy(conflsnap, nvpair_name(snap_pair)); + (void) strlcpy(conflsnap, nvpair_name(snap_pair), + ZFS_MAX_DATASET_NAME_LEN); fnvlist_free(ddpa.err_ds); return (error); diff --git a/module/zfs/dsl_dir.c b/module/zfs/dsl_dir.c index 724f80ff3..205ac1a02 100644 --- a/module/zfs/dsl_dir.c +++ b/module/zfs/dsl_dir.c @@ -234,7 +234,8 @@ dsl_dir_hold_obj(dsl_pool_t *dp, uint64_t ddobj, if (err != 0) goto errout; } else { - (void) strcpy(dd->dd_myname, spa_name(dp->dp_spa)); + (void) strlcpy(dd->dd_myname, spa_name(dp->dp_spa), + sizeof (dd->dd_myname)); } if (dsl_dir_is_clone(dd)) { @@ -394,7 +395,7 @@ getcomponent(const char *path, char *component, const char **nextp) return (SET_ERROR(EINVAL)); if (strlen(path) >= ZFS_MAX_DATASET_NAME_LEN) return (SET_ERROR(ENAMETOOLONG)); - (void) strcpy(component, path); + (void) strlcpy(component, path, ZFS_MAX_DATASET_NAME_LEN); p = NULL; } else if (p[0] == '/') { if (p - path >= ZFS_MAX_DATASET_NAME_LEN) diff --git a/module/zfs/dsl_prop.c b/module/zfs/dsl_prop.c index 784a7308b..3dd959eea 100644 --- a/module/zfs/dsl_prop.c +++ b/module/zfs/dsl_prop.c @@ -130,8 +130,9 @@ dsl_prop_get_dd(dsl_dir_t *dd, const char *propname, if (inheriting) { dsl_dir_name(dd, setpoint); } else { - (void) strcpy(setpoint, - ZPROP_SOURCE_VAL_RECVD); + (void) strlcpy(setpoint, + ZPROP_SOURCE_VAL_RECVD, + MAXNAMELEN); } } break; @@ -204,8 +205,9 @@ dsl_prop_get_ds(dsl_dataset_t *ds, const char *propname, strfree(recvdstr); if (err != ENOENT) { if (setpoint != NULL && err == 0) - (void) strcpy(setpoint, - ZPROP_SOURCE_VAL_RECVD); + (void) strlcpy(setpoint, + ZPROP_SOURCE_VAL_RECVD, + MAXNAMELEN); return (err); } } diff --git a/module/zfs/dsl_scan.c b/module/zfs/dsl_scan.c index 8168a53ca..26900523e 100644 --- a/module/zfs/dsl_scan.c +++ b/module/zfs/dsl_scan.c @@ -358,7 +358,7 @@ scan_init(void) for (int i = 0; i < SPA_DVAS_PER_BP; i++) { char name[36]; - (void) sprintf(name, "sio_cache_%d", i); + (void) snprintf(name, sizeof (name), "sio_cache_%d", i); sio_cache[i] = kmem_cache_create(name, (sizeof (scan_io_t) + ((i + 1) * sizeof (dva_t))), 0, NULL, NULL, NULL, NULL, NULL, 0); diff --git a/module/zfs/dsl_userhold.c b/module/zfs/dsl_userhold.c index 2b2182fad..1a2e2a1a5 100644 --- a/module/zfs/dsl_userhold.c +++ b/module/zfs/dsl_userhold.c @@ -101,9 +101,9 @@ dsl_dataset_user_hold_check(void *arg, dmu_tx_t *tx) size_t len = strlen(nvpair_name(pair)) + strlen(fnvpair_value_string(pair)); char *nameval = kmem_zalloc(len + 2, KM_SLEEP); - (void) strcpy(nameval, nvpair_name(pair)); - (void) strcat(nameval, "@"); - (void) strcat(nameval, fnvpair_value_string(pair)); + (void) strlcpy(nameval, nvpair_name(pair), len + 2); + (void) strlcat(nameval, "@", len + 2); + (void) strlcat(nameval, fnvpair_value_string(pair), len + 2); fnvlist_add_string(tmp_holds, nameval, ""); kmem_free(nameval, len + 2); } diff --git a/module/zfs/spa.c b/module/zfs/spa.c index cf8462cec..4f38d285b 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -6169,8 +6169,8 @@ spa_vdev_attach(spa_t *spa, uint64_t guid, nvlist_t *nvroot, int replacing) spa_strfree(oldvd->vdev_path); oldvd->vdev_path = kmem_alloc(strlen(newvd->vdev_path) + 5, KM_SLEEP); - (void) sprintf(oldvd->vdev_path, "%s/%s", - newvd->vdev_path, "old"); + (void) snprintf(oldvd->vdev_path, strlen(newvd->vdev_path) + 5, + "%s/%s", newvd->vdev_path, "old"); if (oldvd->vdev_devid != NULL) { spa_strfree(oldvd->vdev_devid); oldvd->vdev_devid = NULL; diff --git a/module/zfs/zap.c b/module/zfs/zap.c index 30f62ac43..0d2004651 100644 --- a/module/zfs/zap.c +++ b/module/zfs/zap.c @@ -1030,7 +1030,7 @@ zap_value_search(objset_t *os, uint64_t zapobj, uint64_t value, uint64_t mask, (err = zap_cursor_retrieve(&zc, za)) == 0; zap_cursor_advance(&zc)) { if ((za->za_first_integer & mask) == (value & mask)) { - (void) strcpy(name, za->za_name); + (void) strlcpy(name, za->za_name, MAXNAMELEN); break; } } diff --git a/module/zfs/zap_micro.c b/module/zfs/zap_micro.c index 467812ff6..079c23479 100644 --- a/module/zfs/zap_micro.c +++ b/module/zfs/zap_micro.c @@ -1602,7 +1602,8 @@ zap_cursor_retrieve(zap_cursor_t *zc, zap_attribute_t *za) za->za_integer_length = 8; za->za_num_integers = 1; za->za_first_integer = mzep->mze_value; - (void) strcpy(za->za_name, mzep->mze_name); + (void) strlcpy(za->za_name, mzep->mze_name, + sizeof (za->za_name)); zc->zc_hash = mze->mze_hash; zc->zc_cd = mze->mze_cd; err = 0; diff --git a/module/zfs/zcp_get.c b/module/zfs/zcp_get.c index 42c125d48..35996b8a9 100644 --- a/module/zfs/zcp_get.c +++ b/module/zfs/zcp_get.c @@ -81,13 +81,13 @@ get_objset_type_name(dsl_dataset_t *ds, char *str) return (error); switch (type) { case ZFS_TYPE_SNAPSHOT: - (void) strcpy(str, "snapshot"); + (void) strlcpy(str, "snapshot", ZAP_MAXVALUELEN); break; case ZFS_TYPE_FILESYSTEM: - (void) strcpy(str, "filesystem"); + (void) strlcpy(str, "filesystem", ZAP_MAXVALUELEN); break; case ZFS_TYPE_VOLUME: - (void) strcpy(str, "volume"); + (void) strlcpy(str, "volume", ZAP_MAXVALUELEN); break; default: return (EINVAL); @@ -399,11 +399,11 @@ get_special_prop(lua_State *state, dsl_dataset_t *ds, const char *dsname, break; case ZFS_PROP_FILESYSTEM_COUNT: error = dsl_dir_get_filesystem_count(ds->ds_dir, &numval); - (void) strcpy(setpoint, ""); + (void) strlcpy(setpoint, "", ZFS_MAX_DATASET_NAME_LEN); break; case ZFS_PROP_SNAPSHOT_COUNT: error = dsl_dir_get_snapshot_count(ds->ds_dir, &numval); - (void) strcpy(setpoint, ""); + (void) strlcpy(setpoint, "", ZFS_MAX_DATASET_NAME_LEN); break; case ZFS_PROP_NUMCLONES: numval = dsl_get_numclones(ds); @@ -445,7 +445,8 @@ get_special_prop(lua_State *state, dsl_dataset_t *ds, const char *dsname, sizeof (numval), 1, &numval); } if (error == 0) - (void) strcpy(setpoint, dsname); + (void) strlcpy(setpoint, dsname, + ZFS_MAX_DATASET_NAME_LEN); break; case ZFS_PROP_VOLBLOCKSIZE: { @@ -766,9 +767,10 @@ parse_written_prop(const char *dataset_name, const char *prop_name, ASSERT(zfs_prop_written(prop_name)); const char *name = prop_name + ZFS_WRITTEN_PROP_PREFIX_LEN; if (strchr(name, '@') == NULL) { - (void) sprintf(snap_name, "%s@%s", dataset_name, name); + (void) snprintf(snap_name, ZFS_MAX_DATASET_NAME_LEN, "%s@%s", + dataset_name, name); } else { - (void) strcpy(snap_name, name); + (void) strlcpy(snap_name, name, ZFS_MAX_DATASET_NAME_LEN); } } diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c index f2e808d6f..74741f7d3 100644 --- a/module/zfs/zfs_ioctl.c +++ b/module/zfs/zfs_ioctl.c @@ -2557,7 +2557,8 @@ zfs_prop_set_special(const char *dsname, zprop_source_t source, zfs_cmd_t *zc; zc = kmem_zalloc(sizeof (zfs_cmd_t), KM_SLEEP); - (void) strcpy(zc->zc_name, dsname); + (void) strlcpy(zc->zc_name, dsname, + sizeof (zc->zc_name)); (void) zfs_ioc_userspace_upgrade(zc); (void) zfs_ioc_id_quota_upgrade(zc); kmem_free(zc, sizeof (zfs_cmd_t)); @@ -5069,7 +5070,7 @@ zfs_ioc_recv_new(const char *fsname, nvlist_t *innvl, nvlist_t *outnvl) strchr(snapname, '%')) return (SET_ERROR(EINVAL)); - (void) strcpy(tofs, snapname); + (void) strlcpy(tofs, snapname, sizeof (tofs)); tosnap = strchr(tofs, '@'); *tosnap++ = '\0'; diff --git a/module/zfs/zio.c b/module/zfs/zio.c index 1bd9f2e90..411a42ea5 100644 --- a/module/zfs/zio.c +++ b/module/zfs/zio.c @@ -203,11 +203,13 @@ zio_init(void) if (align != 0) { char name[36]; - (void) sprintf(name, "zio_buf_%lu", (ulong_t)size); + (void) snprintf(name, sizeof (name), "zio_buf_%lu", + (ulong_t)size); zio_buf_cache[c] = kmem_cache_create(name, size, align, NULL, NULL, NULL, NULL, NULL, cflags); - (void) sprintf(name, "zio_data_buf_%lu", (ulong_t)size); + (void) snprintf(name, sizeof (name), "zio_data_buf_%lu", + (ulong_t)size); zio_data_buf_cache[c] = kmem_cache_create(name, size, align, NULL, NULL, NULL, NULL, data_alloc_arena, data_cflags);