On Thu, May 11, 2017 at 10:29:47AM +0300, Gilad Ben-Yossef wrote: > > 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. > Any that are called only by kernel threads would theoretically be safe since kernel threads don't ordinarily receive signals. But I think that at least the drbg and gcm waits can be reached by user threads, since they can be called via algif_rng and algif_aead respectively. I recommend putting any important fixes first, so they can be backported without depending on crypto_wait_req(). Eric
