From 303678350a7253c7bee9d6a3347ee1bcdf9cc177 Mon Sep 17 00:00:00 2001 From: Richard Yao Date: Mon, 28 Nov 2022 16:49:58 -0500 Subject: [PATCH] Convert some sprintf() calls to kmem_scnprintf() These `sprintf()` calls are used repeatedly to write to a buffer. There is no protection against overflow other than reviewers explicitly checking to see if the buffers are big enough. However, such issues are easily missed during review and when they are missed, we would rather stop printing rather than have a buffer overflow, so we convert these functions to use `kmem_scnprintf()`. The Linux kernel provides an entire page for module parameters, so we are safe to write up to PAGE_SIZE. Removing `sprintf()` from these functions removes the last instances of `sprintf()` usage in our platform-independent kernel code. This improves XNU kernel compatibility because the XNU kernel does not support (removed support for?) `sprintf()`. Reviewed-by: Brian Behlendorf Reviewed-by: Jorgen Lundman Signed-off-by: Richard Yao Closes #14209 --- module/icp/algs/aes/aes_impl.c | 6 ++++-- module/icp/algs/blake3/blake3_impl.c | 6 +++--- module/icp/algs/modes/gcm.c | 6 ++++-- module/zcommon/zfs_fletcher.c | 4 ++-- module/zfs/vdev_raidz_math.c | 6 ++++-- 5 files changed, 17 insertions(+), 11 deletions(-) diff --git a/module/icp/algs/aes/aes_impl.c b/module/icp/algs/aes/aes_impl.c index 7c92a7c8d1..9d90914aac 100644 --- a/module/icp/algs/aes/aes_impl.c +++ b/module/icp/algs/aes/aes_impl.c @@ -424,13 +424,15 @@ icp_aes_impl_get(char *buffer, zfs_kernel_param_t *kp) /* list mandatory options */ for (i = 0; i < ARRAY_SIZE(aes_impl_opts); i++) { fmt = (impl == aes_impl_opts[i].sel) ? "[%s] " : "%s "; - cnt += sprintf(buffer + cnt, fmt, aes_impl_opts[i].name); + cnt += kmem_scnprintf(buffer + cnt, PAGE_SIZE - cnt, fmt, + aes_impl_opts[i].name); } /* list all supported implementations */ for (i = 0; i < aes_supp_impl_cnt; i++) { fmt = (i == impl) ? "[%s] " : "%s "; - cnt += sprintf(buffer + cnt, fmt, aes_supp_impl[i]->name); + cnt += kmem_scnprintf(buffer + cnt, PAGE_SIZE - cnt, fmt, + aes_supp_impl[i]->name); } return (cnt); diff --git a/module/icp/algs/blake3/blake3_impl.c b/module/icp/algs/blake3/blake3_impl.c index 1692916cef..7bc4db2c98 100644 --- a/module/icp/algs/blake3/blake3_impl.c +++ b/module/icp/algs/blake3/blake3_impl.c @@ -282,16 +282,16 @@ blake3_param_get(char *buffer, zfs_kernel_param_t *unused) /* cycling */ fmt = IMPL_FMT(impl, IMPL_CYCLE); - cnt += sprintf(buffer + cnt, fmt, "cycle"); + cnt += kmem_scnprintf(buffer + cnt, PAGE_SIZE - cnt, fmt, "cycle"); /* list fastest */ fmt = IMPL_FMT(impl, IMPL_FASTEST); - cnt += sprintf(buffer + cnt, fmt, "fastest"); + cnt += kmem_scnprintf(buffer + cnt, PAGE_SIZE - cnt, fmt, "fastest"); /* list all supported implementations */ for (uint32_t i = 0; i < blake3_supp_impls_cnt; ++i) { fmt = IMPL_FMT(impl, i); - cnt += sprintf(buffer + cnt, fmt, + cnt += kmem_scnprintf(buffer + cnt, PAGE_SIZE - cnt, fmt, blake3_supp_impls[i]->name); } diff --git a/module/icp/algs/modes/gcm.c b/module/icp/algs/modes/gcm.c index 16ef14b8cc..558a578090 100644 --- a/module/icp/algs/modes/gcm.c +++ b/module/icp/algs/modes/gcm.c @@ -1020,13 +1020,15 @@ icp_gcm_impl_get(char *buffer, zfs_kernel_param_t *kp) } #endif fmt = (impl == gcm_impl_opts[i].sel) ? "[%s] " : "%s "; - cnt += sprintf(buffer + cnt, fmt, gcm_impl_opts[i].name); + cnt += kmem_scnprintf(buffer + cnt, PAGE_SIZE - cnt, fmt, + gcm_impl_opts[i].name); } /* list all supported implementations */ for (i = 0; i < gcm_supp_impl_cnt; i++) { fmt = (i == impl) ? "[%s] " : "%s "; - cnt += sprintf(buffer + cnt, fmt, gcm_supp_impl[i]->name); + cnt += kmem_scnprintf(buffer + cnt, PAGE_SIZE - cnt, fmt, + gcm_supp_impl[i]->name); } return (cnt); diff --git a/module/zcommon/zfs_fletcher.c b/module/zcommon/zfs_fletcher.c index 9a201842ea..44c8f486f6 100644 --- a/module/zcommon/zfs_fletcher.c +++ b/module/zcommon/zfs_fletcher.c @@ -903,12 +903,12 @@ fletcher_4_param_get(char *buffer, zfs_kernel_param_t *unused) /* list fastest */ fmt = IMPL_FMT(impl, IMPL_FASTEST); - cnt += sprintf(buffer + cnt, fmt, "fastest"); + cnt += kmem_scnprintf(buffer + cnt, PAGE_SIZE - cnt, fmt, "fastest"); /* list all supported implementations */ for (uint32_t i = 0; i < fletcher_4_supp_impls_cnt; ++i) { fmt = IMPL_FMT(impl, i); - cnt += sprintf(buffer + cnt, fmt, + cnt += kmem_scnprintf(buffer + cnt, PAGE_SIZE - cnt, fmt, fletcher_4_supp_impls[i]->name); } diff --git a/module/zfs/vdev_raidz_math.c b/module/zfs/vdev_raidz_math.c index 2980f8acfb..66f211c430 100644 --- a/module/zfs/vdev_raidz_math.c +++ b/module/zfs/vdev_raidz_math.c @@ -653,13 +653,15 @@ zfs_vdev_raidz_impl_get(char *buffer, zfs_kernel_param_t *kp) /* list mandatory options */ for (i = 0; i < ARRAY_SIZE(math_impl_opts) - 2; i++) { fmt = (impl == math_impl_opts[i].sel) ? "[%s] " : "%s "; - cnt += sprintf(buffer + cnt, fmt, math_impl_opts[i].name); + cnt += kmem_scnprintf(buffer + cnt, PAGE_SIZE - cnt, fmt, + math_impl_opts[i].name); } /* list all supported implementations */ for (i = 0; i < raidz_supp_impl_cnt; i++) { fmt = (i == impl) ? "[%s] " : "%s "; - cnt += sprintf(buffer + cnt, fmt, raidz_supp_impl[i]->name); + cnt += kmem_scnprintf(buffer + cnt, PAGE_SIZE - cnt, fmt, + raidz_supp_impl[i]->name); } return (cnt);