From: Gilad Ben-Yossef <gilad@benyossef.com>
To: Eric Biggers <ebiggers3@gmail.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>,
"David S. Miller" <davem@davemloft.net>,
Jonathan Corbet <corbet@lwn.net>,
David Howells <dhowells@redhat.com>,
Alasdair Kergon <agk@redhat.com>,
Mike Snitzer <snitzer@redhat.com>,
dm-devel@redhat.com, Shaohua Li <shli@kernel.org>,
Steve French <sfrench@samba.org>,
"Theodore Y. Ts'o" <tytso@mit.edu>,
Jaegeuk Kim <jaegeuk@kernel.org>,
Mimi Zohar <zohar@linux.vnet.ibm.com>,
Dmitry Kasatkin <dmitry.kasatkin@gmail.com>,
James Morris <james.l.morris@oracle.com>,
"Serge E. Hallyn" <serge@hallyn.com>,
Ofir Drang <ofir.drang@arm.com>,
Gilad Ben-Yossef <gilad.benyossef@arm.com>,
linux-crypto@vger.kernel.org, linux-doc@vger.kernel.org,
Linux kernel mailing list <linux-kernel@vger.kernel.org>,
keyrings@vger.kernel.org
Subject: Re: [RFC 01/10] crypto: factor async completion for general use
Date: Thu, 11 May 2017 10:29:47 +0300 [thread overview]
Message-ID: <CAOtvUMezWBrHtGxJCBJHVks-XJdSCOMrRj3evsNdY=cB==wc-g@mail.gmail.com> (raw)
In-Reply-To: <20170511035527.GA2936@zzz>
Hi Eric,
On Thu, May 11, 2017 at 6:55 AM, Eric Biggers <ebiggers3@gmail.com> wrote:
> Hi Gilad,
>
> On Sat, May 06, 2017 at 03:59:50PM +0300, Gilad Ben-Yossef wrote:
>> Invoking a possibly async. crypto op and waiting for completion
>> while correctly handling backlog processing is a common task
>> in the crypto API implementation and outside users of it.
>>
>> This patch re-factors one of the in crypto API implementation in
>> preparation for using it across the board instead of hand
>> rolled versions.
>
> Thanks for doing this! It annoyed me too that there wasn't a helper function
> for this. Just a few comments below:
>
Thank you for the review.
I will incorporate the feedback into v2.
...
> With regards to the wait being uninterruptible, I agree that this should be the
> default behavior, because I think users waiting for specific crypto requests are
> generally not prepared to handle the wait actually being interrupted. After
> interruption the crypto operation will still proceed in the background, and it
> will use buffers which the caller has in many cases already freed. However, I'd
> suggest taking a close look at anything that was actually doing an interruptible
> wait before, to see whether it was a bug or intentional (or "doesn't matter").
>
> And yes there could always be a crypto_wait_req_interruptible() introduced if
> some users need it.
So this one was a bit of a shocker. I though the _interruptible use
sites seemed
wrong in the sense of being needless. However, after reading your feedback and
reviewing the code I'm pretty sure every single one of them (including
the one I've
added in dm-verity-target.c this merge window) are down right dangerous and
can cause random data corruption... so thanks for pointing this out!
I though of this patch set as a "make the code pretty" for 4.13 kind
of patch set.
Looks like it's a bug fix now, maybe even stable material.
Anyway, I'll roll a v2 and we'll see.
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
next prev parent reply other threads:[~2017-05-11 7:29 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-06 12:59 [RFC 00/10] introduce crypto wait for async op function Gilad Ben-Yossef
2017-05-06 12:59 ` [RFC 01/10] crypto: factor async completion for general use Gilad Ben-Yossef
2017-05-11 3:55 ` Eric Biggers
2017-05-11 7:29 ` Gilad Ben-Yossef [this message]
2017-05-11 8:09 ` Eric Biggers
2017-05-11 8:55 ` Gilad Ben-Yossef
2017-05-06 12:59 ` [RFC 02/10] crypto: move pub key to generic async completion Gilad Ben-Yossef
2017-05-06 12:59 ` [RFC 03/10] crypto: move drbg " Gilad Ben-Yossef
2017-05-06 12:59 ` [RFC 04/10] crypto: move gcm " Gilad Ben-Yossef
2017-05-06 12:59 ` [RFC 05/10] crypto: move testmgr " Gilad Ben-Yossef
2017-05-06 12:59 ` [RFC 06/10] dm: move dm-verity " Gilad Ben-Yossef
2017-05-06 12:59 ` [RFC 07/10] fscrypt: move " Gilad Ben-Yossef
2017-05-11 4:04 ` Eric Biggers
2017-05-06 12:59 ` [RFC 08/10] cifs: " Gilad Ben-Yossef
2017-05-08 20:56 ` Pavel Shilovsky
2017-05-06 12:59 ` [RFC 09/10] ima: " Gilad Ben-Yossef
2017-05-10 21:26 ` Mimi Zohar
2017-05-06 12:59 ` [RFC 10/10] crypto: adapt api sample to use async. op wait Gilad Ben-Yossef
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAOtvUMezWBrHtGxJCBJHVks-XJdSCOMrRj3evsNdY=cB==wc-g@mail.gmail.com' \
--to=gilad@benyossef.com \
--cc=agk@redhat.com \
--cc=corbet@lwn.net \
--cc=davem@davemloft.net \
--cc=dhowells@redhat.com \
--cc=dm-devel@redhat.com \
--cc=dmitry.kasatkin@gmail.com \
--cc=ebiggers3@gmail.com \
--cc=gilad.benyossef@arm.com \
--cc=herbert@gondor.apana.org.au \
--cc=jaegeuk@kernel.org \
--cc=james.l.morris@oracle.com \
--cc=keyrings@vger.kernel.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ofir.drang@arm.com \
--cc=serge@hallyn.com \
--cc=sfrench@samba.org \
--cc=shli@kernel.org \
--cc=snitzer@redhat.com \
--cc=tytso@mit.edu \
--cc=zohar@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).