Skip to content

Commit

Permalink
ICP: Fix null pointer dereference and use after free
Browse files Browse the repository at this point in the history
In gcm_mode_decrypt_contiguous_blocks(), if vmem_alloc() fails,
bcopy is called with a NULL pointer destination and a length > 0.
This results in undefined behavior. Further ctx->gcm_pt_buf is
freed but not set to NULL, leading to a potential write after
free and a double free due to missing return value handling in
crypto_update_uio(). The code as is may write to ctx->gcm_pt_buf
in gcm_decrypt_final() and may free ctx->gcm_pt_buf again in
aes_decrypt_atomic().

The fix is to slightly rework error handling and check the return
value in crypto_update_uio().

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Tom Caputi <tcaputi@datto.com>
Reviewed-by: Kjeld Schouten <kjeld@schouten-lebbing.nl>
Signed-off-by: Attila Fülöp <attila@fueloep.org>
Closes openzfs#9659
  • Loading branch information
AttilaFueloep authored and behlendorf committed Dec 3, 2019
1 parent 7af7286 commit 54c8366
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 4 deletions.
8 changes: 5 additions & 3 deletions module/icp/algs/modes/gcm.c
Original file line number Diff line number Diff line change
Expand Up @@ -300,11 +300,13 @@ gcm_mode_decrypt_contiguous_blocks(gcm_ctx_t *ctx, char *data, size_t length,
if (length > 0) {
new_len = ctx->gcm_pt_buf_len + length;
new = vmem_alloc(new_len, ctx->gcm_kmflag);
if (new == NULL) {
vmem_free(ctx->gcm_pt_buf, ctx->gcm_pt_buf_len);
ctx->gcm_pt_buf = NULL;
return (CRYPTO_HOST_MEMORY);
}
bcopy(ctx->gcm_pt_buf, new, ctx->gcm_pt_buf_len);
vmem_free(ctx->gcm_pt_buf, ctx->gcm_pt_buf_len);
if (new == NULL)
return (CRYPTO_HOST_MEMORY);

ctx->gcm_pt_buf = new;
ctx->gcm_pt_buf_len = new_len;
bcopy(data, &ctx->gcm_pt_buf[ctx->gcm_processed_data_len],
Expand Down
5 changes: 4 additions & 1 deletion module/icp/core/kcf_prov_lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,12 @@ crypto_update_uio(void *ctx, crypto_data_t *input, crypto_data_t *output,
cur_len = MIN(uiop->uio_iov[vec_idx].iov_len -
offset, length);

(cipher)(ctx, uiop->uio_iov[vec_idx].iov_base + offset,
int rv = (cipher)(ctx, uiop->uio_iov[vec_idx].iov_base + offset,
cur_len, (input == output) ? NULL : output);

if (rv != CRYPTO_SUCCESS) {
return (rv);
}
length -= cur_len;
vec_idx++;
offset = 0;
Expand Down

0 comments on commit 54c8366

Please sign in to comment.