From nobody Sat Jan 15 00:26:44 2022 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 829EF195F991; Sat, 15 Jan 2022 00:26:50 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-qk1-x72b.google.com (mail-qk1-x72b.google.com [IPv6:2607:f8b0:4864:20::72b]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4JbJrT4NMrz4YJN; Sat, 15 Jan 2022 00:26:49 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: by mail-qk1-x72b.google.com with SMTP id b127so12673118qkd.0; Fri, 14 Jan 2022 16:26:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=5xWYrLPmSfGbHL4cv4UEb09n/tfna2mUhUB0qNu04ls=; b=E1yomfpZ/vXIw8xxmHkWjLz3EmogM/QRvUbUjkcylhd+4Rb/XgY0hZhOC3KVRP7mME op1UsQFi3cSzEJKOR8n1SRIFWZwP3lFcATLkePIPBRQnTywTDeDGWTRw9u2UDrKMtBky 2BG5xNtTmWJxmSvJzbEJmd+1t/zYln98/oVII0Y7XaR6/jTNqwlUtG04BPvG7AxRUWTh EZ3O/G6PvMRQ3Sbcor0GLBZfVP1bos7G8kz/ICS5/FmNUvEUedddfmP8O8BCl5pMKrp1 QzaeIwK0Q1FpVy9QBxES2EOSc5eOpSiQYUk4CoEOEuLqLjCrczmD80hmJXr6r//Q/Sf6 cw0Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to; bh=5xWYrLPmSfGbHL4cv4UEb09n/tfna2mUhUB0qNu04ls=; b=R0K/rYJw3YK0khPc2TnKxwNxpPTj9+bKWDzyQpfjhwcwcUZLLHi/xdjzWHJjAO/5ol rWqBL31CRd5HVoCIi3SHZwHtwnqzUGtGPnrt9T2ZU6+hX0xEngvYklbFsOev45TxozJ+ 2DgScNFPCop12ZhIvwgszi6Y2QjTswPdcLZ7ox4nZBtsV6G1l7jD0RkTDcuIpcOKs12H abAFeShsfCOHJSc7ZcABWUz2ZnhKP3Je2tkNL9QH4XKctd2WkpKderHJLruZvmA2hvOx VvokkMIN/j35xpUAjpk+5KjfG3+BckATcbmIaHa13dPByj1NolJAGDrt71opUXs5GyJX 3duA== X-Gm-Message-State: AOAM531Xa1bvqqOrQz3mBxhVkU9Nxjvj7rzb78vPbLo6gZuZfZ8QoXgG YNj3ust/Z59/0TRgmlKbp4tZgTohUNw= X-Google-Smtp-Source: ABdhPJwPOwne3LyD7sX8KDSm40SrPe3yoXWbZ6aFMW2rYl97yodxL7BT2+djVOJIR6iitEx89Cn6bQ== X-Received: by 2002:a05:620a:4495:: with SMTP id x21mr8139996qkp.604.1642206408615; Fri, 14 Jan 2022 16:26:48 -0800 (PST) Received: from nuc (198-84-189-58.cpe.teksavvy.com. [198.84.189.58]) by smtp.gmail.com with ESMTPSA id x14sm4490796qko.110.2022.01.14.16.26.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 14 Jan 2022 16:26:48 -0800 (PST) Sender: Mark Johnston Date: Fri, 14 Jan 2022 19:26:44 -0500 From: Mark Johnston To: Guido Falsi Cc: John Baldwin , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: cfb7b942bed7 - main - cryptosoft: Use multi-block encrypt/decrypt for non-AEAD ciphers. Message-ID: References: <202201112238.20BMcBgx075881@gitrepo.freebsd.org> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 4JbJrT4NMrz4YJN X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=gmail.com header.s=20210112 header.b=E1yomfpZ; dmarc=none; spf=pass (mx1.freebsd.org: domain of markjdb@gmail.com designates 2607:f8b0:4864:20::72b as permitted sender) smtp.mailfrom=markjdb@gmail.com X-Spamd-Result: default: False [-2.70 / 15.00]; RCVD_VIA_SMTP_AUTH(0.00)[]; ARC_NA(0.00)[]; R_DKIM_ALLOW(-0.20)[gmail.com:s=20210112]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; R_SPF_ALLOW(-0.20)[+ip6:2607:f8b0:4000::/36]; MIME_GOOD(-0.10)[text/plain]; MID_RHS_NOT_FQDN(0.50)[]; DMARC_NA(0.00)[freebsd.org]; RCPT_COUNT_FIVE(0.00)[5]; NEURAL_HAM_LONG(-1.00)[-1.000]; RCVD_COUNT_THREE(0.00)[3]; DKIM_TRACE(0.00)[gmail.com:+]; NEURAL_HAM_SHORT(-1.00)[-1.000]; RCVD_IN_DNSWL_NONE(0.00)[2607:f8b0:4864:20::72b:from]; MLMMJ_DEST(0.00)[dev-commits-src-all,dev-commits-src-main]; FORGED_SENDER(0.30)[markj@freebsd.org,markjdb@gmail.com]; MIME_TRACE(0.00)[0:+]; FREEMAIL_ENVFROM(0.00)[gmail.com]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; FROM_NEQ_ENVFROM(0.00)[markj@freebsd.org,markjdb@gmail.com]; RCVD_TLS_ALL(0.00)[]; DWL_DNSWL_NONE(0.00)[gmail.com:dkim] X-ThisMailContainsUnwantedMimeParts: N On Fri, Jan 14, 2022 at 10:27:12PM +0100, Guido Falsi wrote: > On 11/01/22 23:38, John Baldwin wrote: > > The branch main has been updated by jhb: > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=cfb7b942bed72cb798b869d2e36e0097dbd243b2 > > > > commit cfb7b942bed72cb798b869d2e36e0097dbd243b2 > > Author: John Baldwin > > AuthorDate: 2022-01-11 22:18:57 +0000 > > Commit: John Baldwin > > CommitDate: 2022-01-11 22:18:57 +0000 > > > > cryptosoft: Use multi-block encrypt/decrypt for non-AEAD ciphers. > > > > Reviewed by: markj > > Sponsored by: The FreeBSD Foundation > > Differential Revision: https://reviews.freebsd.org/D33531 > > Hi, > > I've just updated to recent head. I have a laptop using ZFS on geli > setup and now it's unable to boot. > > I've seen the failure starting with git revision > 3284f4925f697ad7cc2aa4761ff5cf6ce98fd623 (LRO: Don't merge ACK and > non-ACK packets together - 01/13/22, 17:18) > > it's still there with revision fe453891d7ccc8e173d9293b67f5b4608c5378dd > (01/14/22 11:00:08) > > While a kernel from the binary snapshot downloaded from mirrors compiled > from revision ac413189f53524e489c900b3cfaa80a1552875ca (vfslist.c: > initialize skipvfs variable 01/05/2022) is able to boot correctly. > > The machine panics as soon as it tries to work with geli, this is why I > am replying to this commit message. I'm not completely sure this is the > commit to blame, but it sure is related. > > I have not been able to save the backtrace to file, but the last two > calls are to: > > crypto_cursor_segment() > swcr_encdec() > > so it points to the last part of this patch. I think the problem is that crypto_cursor_segment() doesn't expect to be called once the cursor is at the end of a buffer. It may or may not perform an invalid memory access in that case, depending on the underlying buffer type. Fixing it would complicate the cursor code, maybe it's better to just change cryptosoft to avoid this scenario: diff --git a/sys/opencrypto/cryptosoft.c b/sys/opencrypto/cryptosoft.c index 4d0f7d8718cc..45aa3f41c4b2 100644 --- a/sys/opencrypto/cryptosoft.c +++ b/sys/opencrypto/cryptosoft.c @@ -146,13 +146,11 @@ swcr_encdec(const struct swcr_session *ses, struct cryptop *crp) crypto_cursor_init(&cc_in, &crp->crp_buf); crypto_cursor_advance(&cc_in, crp->crp_payload_start); - inblk = crypto_cursor_segment(&cc_in, &inlen); if (CRYPTO_HAS_OUTPUT_BUFFER(crp)) { crypto_cursor_init(&cc_out, &crp->crp_obuf); crypto_cursor_advance(&cc_out, crp->crp_payload_output_start); } else cc_out = cc_in; - outblk = crypto_cursor_segment(&cc_out, &outlen); encrypting = CRYPTO_OP_IS_ENCRYPT(crp->crp_op); @@ -162,7 +160,13 @@ swcr_encdec(const struct swcr_session *ses, struct cryptop *crp) * 'outlen' is the remaining length of current segment in the * output buffer. */ + inlen = outlen = 0; for (resid = crp->crp_payload_length; resid >= blksz; resid -= todo) { + if (inlen == 0) + inblk = crypto_cursor_segment(&cc_in, &inlen); + if (outlen == 0) + outblk = crypto_cursor_segment(&cc_out, &outlen); + /* * If the current block is not contained within the * current input/output segment, use 'blk' as a local @@ -191,8 +195,6 @@ swcr_encdec(const struct swcr_session *ses, struct cryptop *crp) crypto_cursor_advance(&cc_in, todo); inlen -= todo; inblk += todo; - if (inlen == 0) - inblk = crypto_cursor_segment(&cc_in, &inlen); } if (outblk == blk) { @@ -202,9 +204,6 @@ swcr_encdec(const struct swcr_session *ses, struct cryptop *crp) crypto_cursor_advance(&cc_out, todo); outlen -= todo; outblk += todo; - if (outlen == 0) - outblk = crypto_cursor_segment(&cc_out, - &outlen); } } @@ -476,15 +475,19 @@ swcr_gcm(const struct swcr_session *ses, struct cryptop *crp) /* Do encryption with MAC */ crypto_cursor_init(&cc_in, &crp->crp_buf); crypto_cursor_advance(&cc_in, crp->crp_payload_start); - inblk = crypto_cursor_segment(&cc_in, &inlen); if (CRYPTO_HAS_OUTPUT_BUFFER(crp)) { crypto_cursor_init(&cc_out, &crp->crp_obuf); crypto_cursor_advance(&cc_out, crp->crp_payload_output_start); } else cc_out = cc_in; - outblk = crypto_cursor_segment(&cc_out, &outlen); + inlen = outlen = 0; for (resid = crp->crp_payload_length; resid >= blksz; resid -= todo) { + if (inlen == 0) + inblk = crypto_cursor_segment(&cc_in, &inlen); + if (outlen == 0) + outblk = crypto_cursor_segment(&cc_out, &outlen); + if (inlen < blksz) { crypto_cursor_copydata(&cc_in, blksz, blk); inblk = blk; @@ -510,9 +513,6 @@ swcr_gcm(const struct swcr_session *ses, struct cryptop *crp) crypto_cursor_advance(&cc_out, todo); outlen -= todo; outblk += todo; - if (outlen == 0) - outblk = crypto_cursor_segment(&cc_out, - &outlen); } } else { todo = rounddown2(MIN(resid, inlen), blksz); @@ -525,8 +525,6 @@ swcr_gcm(const struct swcr_session *ses, struct cryptop *crp) crypto_cursor_advance(&cc_in, todo); inlen -= todo; inblk += todo; - if (inlen == 0) - inblk = crypto_cursor_segment(&cc_in, &inlen); } } if (resid > 0) { @@ -563,10 +561,14 @@ swcr_gcm(const struct swcr_session *ses, struct cryptop *crp) /* tag matches, decrypt data */ crypto_cursor_init(&cc_in, &crp->crp_buf); crypto_cursor_advance(&cc_in, crp->crp_payload_start); - inblk = crypto_cursor_segment(&cc_in, &inlen); + inlen = 0; for (resid = crp->crp_payload_length; resid > blksz; resid -= todo) { + if (inlen == 0) + inblk = crypto_cursor_segment(&cc_in, &inlen); + if (outlen == 0) + outblk = crypto_cursor_segment(&cc_out, &outlen); if (inlen < blksz) { crypto_cursor_copydata(&cc_in, blksz, blk); inblk = blk; @@ -588,9 +590,6 @@ swcr_gcm(const struct swcr_session *ses, struct cryptop *crp) crypto_cursor_advance(&cc_in, todo); inlen -= todo; inblk += todo; - if (inlen == 0) - inblk = crypto_cursor_segment(&cc_in, - &inlen); } if (outblk == blk) { @@ -601,9 +600,6 @@ swcr_gcm(const struct swcr_session *ses, struct cryptop *crp) crypto_cursor_advance(&cc_out, todo); outlen -= todo; outblk += todo; - if (outlen == 0) - outblk = crypto_cursor_segment(&cc_out, - &outlen); } } if (resid > 0) { @@ -809,15 +805,19 @@ swcr_ccm(const struct swcr_session *ses, struct cryptop *crp) /* Do encryption/decryption with MAC */ crypto_cursor_init(&cc_in, &crp->crp_buf); crypto_cursor_advance(&cc_in, crp->crp_payload_start); - inblk = crypto_cursor_segment(&cc_in, &inlen); if (CRYPTO_HAS_OUTPUT_BUFFER(crp)) { crypto_cursor_init(&cc_out, &crp->crp_obuf); crypto_cursor_advance(&cc_out, crp->crp_payload_output_start); } else cc_out = cc_in; - outblk = crypto_cursor_segment(&cc_out, &outlen); + inlen = outlen = 0; for (resid = crp->crp_payload_length; resid >= blksz; resid -= todo) { + if (inlen == 0) + inblk = crypto_cursor_segment(&cc_in, &inlen); + if (outlen == 0) + outblk = crypto_cursor_segment(&cc_out, &outlen); + if (inlen < blksz) { crypto_cursor_copydata(&cc_in, blksz, blk); inblk = blk; @@ -843,9 +843,6 @@ swcr_ccm(const struct swcr_session *ses, struct cryptop *crp) crypto_cursor_advance(&cc_out, todo); outlen -= todo; outblk += todo; - if (outlen == 0) - outblk = crypto_cursor_segment(&cc_out, - &outlen); } } else { /* @@ -867,8 +864,6 @@ swcr_ccm(const struct swcr_session *ses, struct cryptop *crp) crypto_cursor_advance(&cc_in, todo); inlen -= todo; inblk += todo; - if (inlen == 0) - inblk = crypto_cursor_segment(&cc_in, &inlen); } } if (resid > 0) { @@ -901,10 +896,16 @@ swcr_ccm(const struct swcr_session *ses, struct cryptop *crp) exf->reinit(ctx, crp->crp_iv, ivlen); crypto_cursor_init(&cc_in, &crp->crp_buf); crypto_cursor_advance(&cc_in, crp->crp_payload_start); - inblk = crypto_cursor_segment(&cc_in, &inlen); + inlen = 0; for (resid = crp->crp_payload_length; resid >= blksz; resid -= todo) { + if (inlen == 0) + inblk = crypto_cursor_segment(&cc_in, &inlen); + if (outlen == 0) + outblk = crypto_cursor_segment(&cc_out, + &outlen); + if (inlen < blksz) { crypto_cursor_copydata(&cc_in, blksz, blk); inblk = blk; @@ -926,9 +927,6 @@ swcr_ccm(const struct swcr_session *ses, struct cryptop *crp) crypto_cursor_advance(&cc_in, todo); inlen -= todo; inblk += todo; - if (inlen == 0) - inblk = crypto_cursor_segment(&cc_in, - &inlen); } if (outblk == blk) { @@ -939,9 +937,6 @@ swcr_ccm(const struct swcr_session *ses, struct cryptop *crp) crypto_cursor_advance(&cc_out, todo); outlen -= todo; outblk += todo; - if (outlen == 0) - outblk = crypto_cursor_segment(&cc_out, - &outlen); } } if (resid > 0) { @@ -1028,6 +1023,12 @@ swcr_chacha20_poly1305(const struct swcr_session *ses, struct cryptop *crp) if (CRYPTO_OP_IS_ENCRYPT(crp->crp_op)) { for (resid = crp->crp_payload_length; resid >= blksz; resid -= todo) { + if (inlen == 0) + inblk = crypto_cursor_segment(&cc_in, &inlen); + if (outlen == 0) + outblk = crypto_cursor_segment(&cc_out, + &outlen); + if (inlen < blksz) { crypto_cursor_copydata(&cc_in, blksz, blk); inblk = blk; @@ -1051,9 +1052,6 @@ swcr_chacha20_poly1305(const struct swcr_session *ses, struct cryptop *crp) crypto_cursor_advance(&cc_in, todo); inlen -= todo; inblk += todo; - if (inlen == 0) - inblk = crypto_cursor_segment(&cc_in, - &inlen); } if (outblk == blk) { @@ -1063,9 +1061,6 @@ swcr_chacha20_poly1305(const struct swcr_session *ses, struct cryptop *crp) crypto_cursor_advance(&cc_out, todo); outlen -= todo; outblk += todo; - if (outlen == 0) - outblk = crypto_cursor_segment(&cc_out, - &outlen); } } if (resid > 0) { @@ -1111,6 +1106,11 @@ swcr_chacha20_poly1305(const struct swcr_session *ses, struct cryptop *crp) for (resid = crp->crp_payload_length; resid > blksz; resid -= todo) { + if (inlen == 0) + inblk = crypto_cursor_segment(&cc_in, &inlen); + if (outlen == 0) + outblk = crypto_cursor_segment(&cc_out, + &outlen); if (inlen < blksz) { crypto_cursor_copydata(&cc_in, blksz, blk); inblk = blk; @@ -1132,9 +1132,6 @@ swcr_chacha20_poly1305(const struct swcr_session *ses, struct cryptop *crp) crypto_cursor_advance(&cc_in, todo); inlen -= todo; inblk += todo; - if (inlen == 0) - inblk = crypto_cursor_segment(&cc_in, - &inlen); } if (outblk == blk) { @@ -1145,9 +1142,6 @@ swcr_chacha20_poly1305(const struct swcr_session *ses, struct cryptop *crp) crypto_cursor_advance(&cc_out, todo); outlen -= todo; outblk += todo; - if (outlen == 0) - outblk = crypto_cursor_segment(&cc_out, - &outlen); } } if (resid > 0) {