Messages in this thread | | | Date | Tue, 24 Oct 2023 10:17:08 +0800 | Subject | Re: [PATCH] net: tls: Fix possible NULL-pointer dereference in tls_decrypt_device() and tls_decrypt_sw() | From | Hangyu Hua <> |
| |
On 23/10/2023 22:03, Sabrina Dubroca wrote: > 2023-10-23, 16:06:11 +0800, Hangyu Hua wrote: >> tls_rx_one_record can be called in tls_sw_splice_read and tls_sw_read_sock >> with msg being NULL. This may lead to null pointer dereferences in >> tls_decrypt_device and tls_decrypt_sw. >> >> Fix this by adding a check. > > Have you actually hit this NULL dereference? I don't see how it can > happen. > > darg->zc is 0 in both cases, so tls_decrypt_device doesn't call > skb_copy_datagram_msg. > > tls_decrypt_sw will call tls_decrypt_sg with out_iov = &msg->msg_iter > (a bogus pointer but no NULL deref yet), and darg->zc is still > 0. tls_decrypt_sg skips the use of out_iov/out_sg and allocates > clear_skb, and the next place where it would use out_iov is skipped > because we have clear_skb.
My bad. I only checked &msg->msg_iter's address in tls_decrypt_sw and found it was wrong. Do I need to make a new patch to fix the harmless bogus pointer?
> > Relevant parts of tls_decrypt_sg: > > static int tls_decrypt_sg(struct sock *sk, struct iov_iter *out_iov, > struct scatterlist *out_sg, > struct tls_decrypt_arg *darg) > { > [...] > if (darg->zc && (out_iov || out_sg)) { > clear_skb = NULL; > [...] > } else { > darg->zc = false; > > clear_skb = tls_alloc_clrtxt_skb(sk, skb, rxm->full_len); > [...] > } > > [...] > if (err < 0) > goto exit_free; > > if (clear_skb) { > sg_init_table(sgout, n_sgout); > sg_set_buf(&sgout[0], dctx->aad, prot->aad_size); > > err = skb_to_sgvec(clear_skb, &sgout[1], prot->prepend_size, > data_len + prot->tail_size); > if (err < 0) > goto exit_free; > } else if (out_iov) { > [...] > } else if (out_sg) { > memcpy(sgout, out_sg, n_sgout * sizeof(*sgout)); > } > [...] > } >
| |