git: 718d4a1d5643 - main - opencrypto: Handle end-of-cursor conditions in crypto_cursor_segment()
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 12 Jun 2023 16:52:39 UTC
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=718d4a1d5643c2faf409001320c3fd64aae57638 commit 718d4a1d5643c2faf409001320c3fd64aae57638 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2023-06-12 16:09:34 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2023-06-12 16:52:24 +0000 opencrypto: Handle end-of-cursor conditions in crypto_cursor_segment() Some consumers, e.g., swcr_encdec(), may call crypto_cursor_segment() after having advanced the cursor to the end of the buffer. In this case I believe the right behaviour is to return NULL and a length of 0. When this occurs with a CRYPTO_BUF_VMPAGE buffer, the cc_vmpage pointer will point past the end of the page pointer array, so crypto_cursor_segment() ends up dereferencing a random pointer before the function returns a length of 0. The uio-backed cursor has a similar problem. Address this by keeping track of the residual buffer length and returning immediately once the length is zero. PR: 271766 Reported by: Andrew "RhodiumToad" Gierth <andrew@tao11.riddles.org.uk> Reviewed by: jhb MFC after: 1 week Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D40428 --- sys/opencrypto/criov.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/sys/opencrypto/criov.c b/sys/opencrypto/criov.c index 5312fe622c5e..c832a9af9eec 100644 --- a/sys/opencrypto/criov.c +++ b/sys/opencrypto/criov.c @@ -321,6 +321,7 @@ crypto_cursor_init(struct crypto_buffer_cursor *cc, break; case CRYPTO_BUF_UIO: cc->cc_iov = cb->cb_uio->uio_iov; + cc->cc_buf_len = cb->cb_uio->uio_resid; break; default: #ifdef INVARIANTS @@ -386,6 +387,7 @@ crypto_cursor_advance(struct crypto_buffer_cursor *cc, size_t amount) cc->cc_offset += amount; break; } + cc->cc_buf_len -= remain; amount -= remain; cc->cc_iov++; cc->cc_offset = 0; @@ -406,14 +408,34 @@ crypto_cursor_segment(struct crypto_buffer_cursor *cc, size_t *len) { switch (cc->cc_type) { case CRYPTO_BUF_CONTIG: - *len = cc->cc_buf_len; - return (cc->cc_buf); + case CRYPTO_BUF_UIO: + case CRYPTO_BUF_VMPAGE: + if (cc->cc_buf_len == 0) { + *len = 0; + return (NULL); + } + break; case CRYPTO_BUF_MBUF: case CRYPTO_BUF_SINGLE_MBUF: if (cc->cc_mbuf == NULL) { *len = 0; return (NULL); } + break; + default: +#ifdef INVARIANTS + panic("%s: invalid buffer type %d", __func__, cc->cc_type); +#endif + *len = 0; + return (NULL); + } + + switch (cc->cc_type) { + case CRYPTO_BUF_CONTIG: + *len = cc->cc_buf_len; + return (cc->cc_buf); + case CRYPTO_BUF_MBUF: + case CRYPTO_BUF_SINGLE_MBUF: if (cc->cc_mbuf->m_flags & M_EXTPG) return (m_epg_segment(cc->cc_mbuf, cc->cc_offset, len)); *len = cc->cc_mbuf->m_len - cc->cc_offset; @@ -426,11 +448,7 @@ crypto_cursor_segment(struct crypto_buffer_cursor *cc, size_t *len) *len = cc->cc_iov->iov_len - cc->cc_offset; return ((char *)cc->cc_iov->iov_base + cc->cc_offset); default: -#ifdef INVARIANTS - panic("%s: invalid buffer type %d", __func__, cc->cc_type); -#endif - *len = 0; - return (NULL); + __assert_unreachable(); } } @@ -504,6 +522,7 @@ crypto_cursor_copyback(struct crypto_buffer_cursor *cc, int size, todo = MIN(remain, size); memcpy(dst, src, todo); src += todo; + cc->cc_buf_len -= todo; if (todo < remain) { cc->cc_offset += todo; break; @@ -593,6 +612,7 @@ crypto_cursor_copydata(struct crypto_buffer_cursor *cc, int size, void *vdst) todo = MIN(remain, size); memcpy(dst, src, todo); dst += todo; + cc->cc_buf_len -= todo; if (todo < remain) { cc->cc_offset += todo; break;