HomeFreeBSD

Cleanup: Switch to strlcpy from strncpy

Description

Cleanup: Switch to strlcpy from strncpy

Coverity found a bug in zfs_secpolicy_create_clone() where it is
possible for us to pass an unterminated string when zfs_get_parent()
returns an error. Upon inspection, it is clear that using strlcpy()
would have avoided this issue.

Looking at the codebase, there are a number of other uses of strncpy()
that are unsafe and even when it is used safely, switching to
strlcpy() would make the code more readable. Therefore, we switch all
instances where we use strncpy() to use strlcpy().

Unfortunately, we do not portably have access to strlcpy() in
tests/zfs-tests/cmd/zfs_diff-socket.c because it does not link to
libspl. Modifying the appropriate Makefile.am to try to link to it
resulted in an error from the naming choice used in the file. Trying to
disable the check on the file did not work on FreeBSD because Clang
ignores #undef when a definition is provided by -Dstrncpy(...)=....
We workaround that by explictly including the C file from libspl into
the test. This makes things build correctly everywhere.

We add a deprecation warning to config/Rules.am and suppress it on the
remaining strncpy() usage. strlcpy() is not portably avaliable in
tests/zfs-tests/cmd/zfs_diff-socket.c, so we use snprintf() there as a
substitute.

This patch does not tackle the related problem of strcpy(), which is
even less safe. Thankfully, a quick inspection found that it is used far
more correctly than strncpy() was used. A quick inspection did not find
any problems with strcpy() usage outside of zhack, but it should be
said that I only checked around 90% of them.

Lastly, some of the fields in kstat_t varied in size by 1 depending on
whether they were in userspace or in the kernel. The origin of this
discrepancy appears to be 04a479f7066ccdaa23a6546955303b172f4a6909 where
it was made for no apparent reason. It conflicts with the comment on
KSTAT_STRLEN, so we shrink the kernel field sizes to match the userspace
field sizes.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #13876

Details

Provenance
Richard Yao <richard.yao@alumni.stonybrook.edu>Authored on Sep 27 2022, 11:35 PM
GitHub <noreply@github.com>Committed on Sep 27 2022, 11:35 PM
Parents
rG3ed9d6883bcf: Enforce "-F" flag on resuming recv of full/newfs on existing dataset
Branches
Unknown
Tags
Unknown

Event Timeline

GitHub <noreply@github.com> committed rG7584fbe846b4: Cleanup: Switch to strlcpy from strncpy (authored by Richard Yao <richard.yao@alumni.stonybrook.edu>).Sep 27 2022, 11:35 PM