Skip to content

Commit

Permalink
crypto_get_ptrs() should always write to *out_data_2
Browse files Browse the repository at this point in the history
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 <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes openzfs#14043
  • Loading branch information
ryao authored and andrewc12 committed Oct 21, 2022
1 parent 3d022c9 commit e463e7b
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion module/icp/algs/modes/modes.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit e463e7b

Please sign in to comment.