Introduce kmem_scnprintf()

`snprintf()` is meant to protect against buffer overflows, but operating
on the buffer using its return value, possibly by calling it again, can
cause a buffer overflow, because it will return how many characters it
would have written if it had enough space even when it did not. In a
number of places, we repeatedly call snprintf() by successively
incrementing a buffer offset and decrementing a buffer length, by its
return value. This is a potentially unsafe usage of `snprintf()`
whenever the buffer length is reached. CodeQL complained about this.

To fix this, we introduce `kmem_scnprintf()`, which will return 0 when
the buffer is zero or the number of written characters, minus 1 to
exclude the NULL character, when the buffer was too small. In all other
cases, it behaves like snprintf(). The name is inspired by the Linux and
XNU kernels' `scnprintf()`. The implementation was written before I
thought to look at `scnprintf()` and had a good name for it, but it
turned out to have identical semantics to the Linux kernel version.
That lead to the name, `kmem_scnprintf()`.

CodeQL only catches this issue in loops, so repeated use of snprintf()
outside of a loop was not caught. As a result, a thorough audit of the
codebase was done to examine all instances of `snprintf()` usage for
potential problems and a few were caught. Fixes for them are included in
this patch.

Unfortunately, ZED is one of the places where `snprintf()` is
potentially used incorrectly. Since using `kmem_scnprintf()` in it would
require changing how it is linked, we modify its usage to make it safe,
no matter what buffer length is used. In addition, there was a bug in
the use of the return value where the NULL format character was not
being written by pwrite(). That has been fixed.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Closes #14098

1 files changed, 19 insertions, 19 deletions

diff --git a/module/zfs/zfs_chksum.c b/module/zfs/zfs_chksum.c
index 74b4cb8d2..4a9a36d87 100644
--- a/module/zfs/zfs_chksum.c
+++ b/module/zfs/zfs_chksum.c
@@ -81,15 +81,15 @@ chksum_kstat_headers(char *buf, size_t size)
 {
 	ssize_t off = 0;
 
-	off += snprintf(buf + off, size, "%-23s", "implementation");
-	off += snprintf(buf + off, size - off, "%8s", "1k");
-	off += snprintf(buf + off, size - off, "%8s", "4k");
-	off += snprintf(buf + off, size - off, "%8s", "16k");
-	off += snprintf(buf + off, size - off, "%8s", "64k");
-	off += snprintf(buf + off, size - off, "%8s", "256k");
-	off += snprintf(buf + off, size - off, "%8s", "1m");
-	off += snprintf(buf + off, size - off, "%8s", "4m");
-	(void) snprintf(buf + off, size - off, "%8s\n", "16m");
+	off += kmem_scnprintf(buf + off, size, "%-23s", "implementation");
+	off += kmem_scnprintf(buf + off, size - off, "%8s", "1k");
+	off += kmem_scnprintf(buf + off, size - off, "%8s", "4k");
+	off += kmem_scnprintf(buf + off, size - off, "%8s", "16k");
+	off += kmem_scnprintf(buf + off, size - off, "%8s", "64k");
+	off += kmem_scnprintf(buf + off, size - off, "%8s", "256k");
+	off += kmem_scnprintf(buf + off, size - off, "%8s", "1m");
+	off += kmem_scnprintf(buf + off, size - off, "%8s", "4m");
+	(void) kmem_scnprintf(buf + off, size - off, "%8s\n", "16m");
 
 	return (0);
 }
@@ -102,23 +102,23 @@ chksum_kstat_data(char *buf, size_t size, void *data)
 	char b[24];
 
 	cs = (chksum_stat_t *)data;
-	snprintf(b, 23, "%s-%s", cs->name, cs->impl);
-	off += snprintf(buf + off, size - off, "%-23s", b);
-	off += snprintf(buf + off, size - off, "%8llu",
+	kmem_scnprintf(b, 23, "%s-%s", cs->name, cs->impl);
+	off += kmem_scnprintf(buf + off, size - off, "%-23s", b);
+	off += kmem_scnprintf(buf + off, size - off, "%8llu",
 	    (u_longlong_t)cs->bs1k);
-	off += snprintf(buf + off, size - off, "%8llu",
+	off += kmem_scnprintf(buf + off, size - off, "%8llu",
 	    (u_longlong_t)cs->bs4k);
-	off += snprintf(buf + off, size - off, "%8llu",
+	off += kmem_scnprintf(buf + off, size - off, "%8llu",
 	    (u_longlong_t)cs->bs16k);
-	off += snprintf(buf + off, size - off, "%8llu",
+	off += kmem_scnprintf(buf + off, size - off, "%8llu",
 	    (u_longlong_t)cs->bs64k);
-	off += snprintf(buf + off, size - off, "%8llu",
+	off += kmem_scnprintf(buf + off, size - off, "%8llu",
 	    (u_longlong_t)cs->bs256k);
-	off += snprintf(buf + off, size - off, "%8llu",
+	off += kmem_scnprintf(buf + off, size - off, "%8llu",
 	    (u_longlong_t)cs->bs1m);
-	off += snprintf(buf + off, size - off, "%8llu",
+	off += kmem_scnprintf(buf + off, size - off, "%8llu",
 	    (u_longlong_t)cs->bs4m);
-	(void) snprintf(buf + off, size - off, "%8llu\n",
+	(void) kmem_scnprintf(buf + off, size - off, "%8llu\n",
 	    (u_longlong_t)cs->bs16m);
 
 	return (0);