From fd0e562f2238b45da49d3c70274eef64d651e3c7 Mon Sep 17 00:00:00 2001 From: Richard Yao Date: Sat, 15 Oct 2022 23:35:56 -0400 Subject: [PATCH] crypto_get_ptrs() should always write to *out_data_2 Callers will check if it has been set to NULL before trying to access it, but never initialize it themselves. Whenever "one block spans two iovecs", `crypto_get_ptrs()` will return, without ever setting `*out_data_2 = NULL`. The caller will then do a NULL check against the uninitailized pointer and if it is not zero, pass it to `memcpy()`. The only reason this has not caused horrible runtime issues is because `memcpy()` should be told to copy zero bytes when this happens. That said, this is technically undefined behavior, so we should correct it so that future changes to the code cannot trigger it. Clang's static analyzer found this with the help of CodeChecker's CTU analysis. Reviewed-by: Brian Behlendorf Signed-off-by: Richard Yao Closes #14043 --- module/icp/algs/modes/modes.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/module/icp/algs/modes/modes.c b/module/icp/algs/modes/modes.c index b98db0ac14ec..2d1b5ff1a919 100644 --- a/module/icp/algs/modes/modes.c +++ b/module/icp/algs/modes/modes.c @@ -106,8 +106,10 @@ crypto_get_ptrs(crypto_data_t *out, void **iov_or_mp, offset_t *current_offset, } else { /* one block spans two iovecs */ *out_data_1_len = iov_len - offset; - if (vec_idx == zfs_uio_iovcnt(uio)) + if (vec_idx == zfs_uio_iovcnt(uio)) { + *out_data_2 = NULL; return; + } vec_idx++; zfs_uio_iov_at_index(uio, vec_idx, &iov_base, &iov_len); *out_data_2 = (uint8_t *)iov_base;