Re: Bug of dm-crypt?

From: Huang Ying
Date: Fri Feb 27 2009 - 04:47:14 EST


Hi, Milan,

On Fri, 2009-02-27 at 16:41 +0800, Milan Broz wrote:
> Herbert Xu wrote:
> > On Fri, Feb 27, 2009 at 01:31:56PM +0800, Huang Ying wrote:
> >> I had ever heard from you that the only thing guaranteed in the
> >> completion function of async ablkcipher cryption is the req->data has
> >> the value you set before. The request pointer itself may be changed. But
> >> in dm-crypt, I found they rely on request pointer in completion
> >> function: kcryptd_async_done. This makes my AES-NI cryptd usage panic.
> >>
> >> Do you think that is a bug?
> >
> > Absolutely.
> >
> > It should use cc->req instead.
>
> ok, I'll check it and try to fix that.
>
> Can you send me backtrace from that panic?

The panic backtrace is just some random kernel page fault or general
protection fault, because the freed pointer is not the allocated one.

> What's wrong in async callback now - mempool_free using async_req directly?

Yes. I have a fix by storing real allocated async_req in
async_req->data. Can you help me to review that?

> (Btw in some previous discussuion I asked if cryptd can be used
> for some kind of parallel speedup for dm-crypt in async mode and response
> was that cryptd "is only meant to be a demo showing how an async
> implementation should be written".
>
> So this changed to real working thread for AES-NI?)

Yes. Now AES-NI uses it. And because a Per-CPU thread implementation is
used for cryptd, it can be used to speedup dm-crypt in some situation
too I think.

Best Regards,
Huang Ying

Attachment: signature.asc
Description: This is a digitally signed message part