Date: Fri, 29 May 2020 07:45:37 -0700 From: John Baldwin <jhb@FreeBSD.org> To: =?UTF-8?Q?Kornel_Dul=c4=99ba?= <mindal@semihalf.com> Cc: Marcin Wojtas <mw@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r361583 - head/sys/crypto/aesni Message-ID: <e17450f6-cfae-7485-e872-f780ea26512e@FreeBSD.org> In-Reply-To: <CAKpxNizeGuQf-CuGMW8u4JF9=_kTc0y=sqFYen7NPBHd-auV9g@mail.gmail.com> References: <202005280913.04S9DKWv013795@repo.freebsd.org> <9fdb00be-00e0-6aff-51ad-7a84b4215a4e@FreeBSD.org> <CAKpxNizeGuQf-CuGMW8u4JF9=_kTc0y=sqFYen7NPBHd-auV9g@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 5/29/20 12:46 AM, Kornel Dulęba wrote: > On Thu, May 28, 2020 at 8:30 PM John Baldwin <jhb@freebsd.org> wrote: > >> On 5/28/20 2:13 AM, Marcin Wojtas wrote: >>> Author: mw >>> Date: Thu May 28 09:13:20 2020 >>> New Revision: 361583 >>> URL: https://svnweb.freebsd.org/changeset/base/361583 >>> >>> Log: >>> Change return types of hash update functions in SHA-NI >>> >>> r359374 introduced crypto_apply function which takes as argument a >> function pointer >>> that is expected to return an int, however aesni hash update functions >>> return void. >>> Because of that the function pointer passed was simply cast with >>> its return value changed. >>> This resulted in undefined behavior, in particular when mbuf is used, >> (ipsec) >>> m_apply checks return value of function pointer passed to it >>> and in our case bogusly fails after calculating hash of the first mbuf >>> in chain. >>> Fix it by changing signatures of sha update routines in aesni and >>> dropping the casts. >> >> Hmm, I missed one nit in the review. r359374 didn't introduce >> crypto_apply, it just changed some of the arguments arguments (crp >> instead of crp_buf and crp_flags). This fix needs to be MFC'd to 12 >> as well since the issue with the return type is also present there. >> >> -- >> John Baldwin >> > > Whoops, I should have read the diff more carefully. > Before debugging I did some bisecting and found r359374 as the culprit. > In r359374 return types of intel_sha256_update and intel_sha1_update were > changed to void which is the root cause. > SHA-NI on stable/12 should work just fine right now. > Sorry for the noise. Hmm, I did change the return types, oof. Thanks for tracking this down. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?e17450f6-cfae-7485-e872-f780ea26512e>