Hi Herbert,
The AIO support for algif_aead is broken when submitting more than one iocb.
The break happens in aead_recvmsg_async at the following code:
/* ensure output buffer is sufficiently large */
if (usedpages < outlen)
goto free;
The reason is that when submitting, say, two iocb, ctx->used contains the
buffer length for two AEAD operations (as expected). However, the recvmsg code
is invoked for each iocb individually and thus usedpages should only be
expected to point to memory for one AEAD operation. But this violates the
check above.
For example, I have two independent AEAD operations that I want to trigger.
The input to each is 48 bytes (including space for AAD and tag). The output
buffer that I have for each AEAD operation is also 48 bytes and thus
sufficient for the AEAD operation. Yet, when submitting the two AEAD
operations in one io_submit (i.e. using two iocb), ctx->used indicates that
the kernel has 96 bytes to process. This is correct, but only half of it
should be processed in one recvmsg_async invocation.
Note, the AIO operation works perfectly well, when io_submit only sends one
iocb.
Do you have any idea on how to fix that?
Ciao
Stephan
> The AIO support for algif_aead is broken when submitting more than one iocb.
> The break happens in aead_recvmsg_async at the following code:
>
I think the kernel needs to take a half step back, and add the missing
self tests and test cases to be more proactive in detecting breaks
earlier. Speaking first hand, some of these breaks have existed for
months.
I don't take the position you can't break things. I believe you can't
make an omelet without breaking eggs; and if you're not breaking
something, then you're probably not getting anything done. The
engineering defect is not detecting the break.
Jeff
Am Sonntag, 11. September 2016, 08:43:00 CEST schrieb Jeffrey Walton:
Hi Jeffrey,
> > The AIO support for algif_aead is broken when submitting more than one
> > iocb.
> > The break happens in aead_recvmsg_async at the following code:
> I think the kernel needs to take a half step back, and add the missing
> self tests and test cases to be more proactive in detecting breaks
> earlier. Speaking first hand, some of these breaks have existed for
> months.
>
> I don't take the position you can't break things. I believe you can't
> make an omelet without breaking eggs; and if you're not breaking
> something, then you're probably not getting anything done. The
> engineering defect is not detecting the break.
The testing that is implemented for libkcapi should cover almost all code
paths of AF_ALG in the kernel. However, I just added the AIO support to the
library in the last few days as this logic is not straight forward. Thus these
issues show up now.
If you wish to analyze the AIO support more, I can certainly push my current
development branch of libkcapi to my github tree so that you would have a
working AIO user space component.
Ciao
Stephan
On Sun, Sep 11, 2016 at 04:59:19AM +0200, Stephan Mueller wrote:
> Hi Herbert,
>
> The AIO support for algif_aead is broken when submitting more than one iocb.
> The break happens in aead_recvmsg_async at the following code:
>
> /* ensure output buffer is sufficiently large */
> if (usedpages < outlen)
> goto free;
>
> The reason is that when submitting, say, two iocb, ctx->used contains the
> buffer length for two AEAD operations (as expected). However, the recvmsg code
I don't think we should allow that. We should make it so that you
must start a recvmsg before you can send data for a new request.
Remember that the async path should be identical to the sync path,
except that you don't wait for completion.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Am Dienstag, 13. September 2016, 18:12:46 CEST schrieb Herbert Xu:
Hi Herbert,
> I don't think we should allow that. We should make it so that you
> must start a recvmsg before you can send data for a new request.
>
> Remember that the async path should be identical to the sync path,
> except that you don't wait for completion.
The question is, how does the algif code knows when more than one iocb was
submitted? Note, each iocb is translated into an independent call of the
recvmsg_async.
Ciao
Stephan
Am Dienstag, 13. September 2016, 18:12:46 CET schrieb Herbert Xu:
Hi Herbert,
> On Sun, Sep 11, 2016 at 04:59:19AM +0200, Stephan Mueller wrote:
> > Hi Herbert,
> >
> > The AIO support for algif_aead is broken when submitting more than one
> > iocb.>
> > The break happens in aead_recvmsg_async at the following code:
> > /* ensure output buffer is sufficiently large */
> > if (usedpages < outlen)
> >
> > goto free;
> >
> > The reason is that when submitting, say, two iocb, ctx->used contains the
> > buffer length for two AEAD operations (as expected). However, the recvmsg
> > code
> I don't think we should allow that. We should make it so that you
> must start a recvmsg before you can send data for a new request.
>
> Remember that the async path should be identical to the sync path,
> except that you don't wait for completion.
Just as a followup: with the patch submitted the other day to cover the AAD
and tag handling, the algif_aead now supports also multiple iocb.
Ciao
Stephan