Re: [pkg-cryptsetup-devel] Bug#541835: crypto configuration / dependencies broken

2009-09-02 Thread Herbert Xu
Jonas Meurer  wrote:
> 
> i guess the best solution would be to improve the module dependency
> system. cipher meta-modules like 'aes' should depend on all available
> implementations. same for ivciphers, hashs, rng implementations etc.

We already do that for algorithms at least, they should all have
aliases of the form "ALG" or "ALG-all".

Things like chainiv/eseqiv should just be included always.  There
are unlikely to be any new algorithms of that type.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Crypto oops in async_chainiv_do_postponed

2009-09-02 Thread Herbert Xu
On Wed, Sep 02, 2009 at 06:47:49PM -0500, Brad Bosch wrote:
>
> Assume the worker thread is executing between the dequeue in
> async_chainiv_do_postponed and the clear_bit call in
> async_chainiv_schedule_work.  Further assume that we are processing
> the last item on the queue so durring this time, ctx->queue.qlen =
> 0.  **INUSE is still set at this point.
> 
> Meanwhile, three threads enter async_chainiv_givencrypt for the same
> ctx at about the same time.
> 
> Thread one calls test_and_set_bit which returns 1 and calls
> async_cahiniv_postpone_request but suppose it has not yet enqueued.
> Now INUSE is set and qlen=0.
> 
> Next, the worker thread calls clear_bit in async_chainiv_schedule_work
> but it is interrupted before it can call test_and_set_bit.  Now INUSE
> is clear and qlen=0
> 
> The test_and_set_bit in thread two is called at this moment and
> returns 0 and then calls async_chainiv_givencrypt_tail.  Now INUSE is
> set and qlen=0.
> 
> Thread one now locks the ctx and calls skcipher_enqueue_givcrypt and
> unlocks.  Now INUSE is set and qlen=1.
> 
> Thread three calls test_and_set_bit which returns 1 and then it clears
> INUSE since qlen=1 and it calls postpone with INUSE clear and qlen=1

How can thread three clear INUSE if test_and_set_bit returned 1?
If thread three sees it set then it will postpone.  It can only
clear it if it was not set originally.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Crypto oops in async_chainiv_do_postponed

2009-09-02 Thread Brad Bosch
Herbert Xu writes:
 > On Wed, Sep 02, 2009 at 09:08:38AM -0500, Brad Bosch wrote:
 > > 
 > > Assume the worker thread is executing between the dequeue in
 > > async_chainiv_do_postponed and the clear_bit call in
 > > async_chainiv_schedule_work.  Further assume that we are processing
 > 
 > It cannot.  The worker thread can only execute when it owns
 > the INUSE bit.  In that case do_postponed will never call the
 > schedule_work function.

In the example I cited (one entry in the queue when the worker
function starts), async_chainiv_schedule_work is indeed executed.
(indirectly) by async_chainiv_givencrypt_tail from the worker thread.
I'm sorry I didn't make it more clear that it is that code path I was
talking about.

 > 
 > Perhaps you were misled by the clear_bit call in schedule_work.
 > That is only used if we end up not scheduling the work.

No, I was not misled.  But apparently, I was not clear.  I do
understand how you use the INUSE bit.  I did not say above that
INUSE is not set when the worker thread is running (at least not for
the first part of my example).  If you had read further, you might
have noticed that the following paragraphs showed that indeed I do
understand that INUSE is set in the worker thread as evidenced by
"thread one calls test_and_set_bit which returns 1" I have added one
sentence (marked by **) to my event description below to make my
understanding more clear.  Please read on.

Assume the worker thread is executing between the dequeue in
async_chainiv_do_postponed and the clear_bit call in
async_chainiv_schedule_work.  Further assume that we are processing
the last item on the queue so durring this time, ctx->queue.qlen =
0.  **INUSE is still set at this point.

Meanwhile, three threads enter async_chainiv_givencrypt for the same
ctx at about the same time.

Thread one calls test_and_set_bit which returns 1 and calls
async_cahiniv_postpone_request but suppose it has not yet enqueued.
Now INUSE is set and qlen=0.

Next, the worker thread calls clear_bit in async_chainiv_schedule_work
but it is interrupted before it can call test_and_set_bit.  Now INUSE
is clear and qlen=0

The test_and_set_bit in thread two is called at this moment and
returns 0 and then calls async_chainiv_givencrypt_tail.  Now INUSE is
set and qlen=0.

Thread one now locks the ctx and calls skcipher_enqueue_givcrypt and
unlocks.  Now INUSE is set and qlen=1.

Thread three calls test_and_set_bit which returns 1 and then it clears
INUSE since qlen=1 and it calls postpone with INUSE clear and qlen=1

Now thread three will use ctx->err to hold the return value of
skcipher_enqueue_givcrypt at the same time as thread two uses ctx->err
to hold the return value of crypto_ablkcipher_encrypt!

Did I make a mistake above?  I suspect more bad things can happen as
well in this scenario, but I'm just focusing on the use of ctx->err here.

Thanks

--Brad
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Crypto Fixes for 2.6.31

2009-09-02 Thread Herbert Xu
Hi Linus:

This push fixes a serious regression for IPsec when using the
chainiv algorithm.  We were checking for NULL after converting a
pointer that can be NULL to its container, which means that
the NULL pointer check is useless.  This would occur when the
chainiv backlog queue is depleted.  The result is a crash.

Based on the one report received it does not occur all the time
though, possibly because we only use the backlog when two CPUs
try to push data through a single SA at the same time, which is
rare.
 
Please pull from

git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git

or

master.kernel.org:/pub/scm/linux/kernel/git/herbert/crypto-2.6.git


Herbert Xu (1):
  crypto: skcipher - Fix skcipher_dequeue_givcrypt NULL test

 crypto/algapi.c|   11 +--
 include/crypto/algapi.h|1 +
 include/crypto/internal/skcipher.h |4 ++--
 3 files changed, 12 insertions(+), 4 deletions(-)

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Crypto oops in async_chainiv_do_postponed

2009-09-02 Thread Herbert Xu
On Wed, Sep 02, 2009 at 09:08:38AM -0500, Brad Bosch wrote:
> 
> Assume the worker thread is executing between the dequeue in
> async_chainiv_do_postponed and the clear_bit call in
> async_chainiv_schedule_work.  Further assume that we are processing

It cannot.  The worker thread can only execute when it owns
the INUSE bit.  In that case do_postponed will never call the
schedule_work function.

Perhaps you were misled by the clear_bit call in schedule_work.
That is only used if we end up not scheduling the work.
 
> Unfortunately, the offset problem is not easily reproduced with our
> application, so testing long enough to be sure the problem is fixed
> (assuming that it was indeed the cause of the oops) may not be
> practical.  All I can say at the moment is that I have not seen the
> crash since I introduced the two patches I sent you.

OK I'll forward this upstream then.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Crypto oops in async_chainiv_do_postponed

2009-09-02 Thread Brad Bosch
(resent due to bounce notification for vger)
Herbert Xu writes:
 > On Tue, Sep 01, 2009 at 10:42:44AM -0500, Brad Bosch wrote:
 > > 
 > > Now, ctx-err may be used by both async_chainiv_postpone_request to
 > > store the return value from skcipher_enqueue_givcrypt and by
 > > async_chainiv_givencrypt_tail to store the return value from
 > > crypto_ablkcipher_encrypt at the same time.  This can cause the
 > > calling function to think async_chainiv_givencrypt has completed it's
 > > work, when in fact, the work was defered.
 > 
 > async_chainiv_postpone_request never touches ctx->err unless
 > it can obtain the INUSE bit lock.  On the other hand, the normal
 > patch async_chainiv_givencrypt_tail never relinquishes the INUSE
 > bit until it is finisehd with ctx->err.

But the above statements are not adequate to demonstrate that your use
of the INUSE flag always prevents a condition where both
async_chainiv_postpone_request and async_chainiv_givencrypt_tail
operate on the same ctx at the same time.  The flaw in your logic may
be that async_chainiv_schedule_work does not have solid assurance that
it's thread is the one that holds the INUSE bit when it calls
clear_bit.

I seem to have trouble getting the details right in describing a path
that causes both uses of ctx->err to happen at the same time.  Let me
try again.

Assume the worker thread is executing between the dequeue in
async_chainiv_do_postponed and the clear_bit call in
async_chainiv_schedule_work.  Further assume that we are processing
the last item on the queue so durring this time, ctx->queue.qlen =
0.

Meanwhile, three threads enter async_chainiv_givencrypt for the same
ctx at about the same time.

Thread one calls test_and_set_bit which returns 1 and calls
async_cahiniv_postpone_request but suppose it has not yet enqueued.
Now INUSE is set and qlen=0.

Next, the worker thread calls clear_bit in async_chainiv_schedule_work
but it is interrupted before it can call test_and_set_bit.  Now INUSE
is clear and qlen=0

The test_and_set_bit in thread two is called at this moment and
returns 0 and then calls async_chainiv_givencrypt_tail.  Now INUSE is
set and qlen=0.

Thread one now locks the ctx and calls skcipher_enqueue_givcrypt and
unlocks.  Now INUSE is set and qlen=1.

Thread three calls test_and_set_bit which returns 1 and then it clears
INUSE since qlen=1 and it calls postpone with INUSE clear and qlen=1

Now thread three will use ctx->err to hold the return value of
skcipher_enqueue_givcrypt at the same time as thread two uses ctx->err
to hold the return value of crypto_ablkcipher_encrypt!

Did I make a mistake above?  I suspect more bad things can happen as
well in this scenario, but I'm just focusing on the use of ctx->err here.

 > 
 > Please let me know whether it actually fixes your problem though
 > so I can get this upstream.

Unfortunately, the offset problem is not easily reproduced with our
application, so testing long enough to be sure the problem is fixed
(assuming that it was indeed the cause of the oops) may not be
practical.  All I can say at the moment is that I have not seen the
crash since I introduced the two patches I sent you.

Thanks for taking the time to discuss this!

--Brad
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Crypto oops in async_chainiv_do_postponed

2009-09-02 Thread Brad Bosch
Herbert Xu writes:
 > On Tue, Sep 01, 2009 at 10:42:44AM -0500, Brad Bosch wrote:
 > > 
 > > Now, ctx-err may be used by both async_chainiv_postpone_request to
 > > store the return value from skcipher_enqueue_givcrypt and by
 > > async_chainiv_givencrypt_tail to store the return value from
 > > crypto_ablkcipher_encrypt at the same time.  This can cause the
 > > calling function to think async_chainiv_givencrypt has completed it's
 > > work, when in fact, the work was defered.
 > 
 > async_chainiv_postpone_request never touches ctx->err unless
 > it can obtain the INUSE bit lock.  On the other hand, the normal
 > patch async_chainiv_givencrypt_tail never relinquishes the INUSE
 > bit until it is finisehd with ctx->err.

But the above statements are not adequate to demonstrate that your use
of the INUSE flag always prevents a condition where both
async_chainiv_postpone_request and async_chainiv_givencrypt_tail
operate on the same ctx at the same time.  The flaw in your logic may
be that async_chainiv_schedule_work does not have solid assurance that
it's thread is the one that holds the INUSE bit when it calls
clear_bit.

I seem to have trouble getting the details right in describing a path
that causes both uses of ctx->err to happen at the same time.  Let me
try again.

Assume the worker thread is executing between the dequeue in
async_chainiv_do_postponed and the clear_bit call in
async_chainiv_schedule_work.  Further assume that we are processing
the last item on the queue so durring this time, ctx->queue.qlen =
0.

Meanwhile, three threads enter async_chainiv_givencrypt for the same
ctx at about the same time.

Thread one calls test_and_set_bit which returns 1 and calls
async_cahiniv_postpone_request but suppose it has not yet enqueued.
Now INUSE is set and qlen=0.

Next, the worker thread calls clear_bit in async_chainiv_schedule_work
but it is interrupted before it can call test_and_set_bit.  Now INUSE
is clear and qlen=0

The test_and_set_bit in thread two is called at this moment and
returns 0 and then calls async_chainiv_givencrypt_tail.  Now INUSE is
set and qlen=0.

Thread one now locks the ctx and calls skcipher_enqueue_givcrypt and
unlocks.  Now INUSE is set and qlen=1.

Thread three calls test_and_set_bit which returns 1 and then it clears
INUSE since qlen=1 and it calls postpone with INUSE clear and qlen=1

Now thread three will use ctx->err to hold the return value of
skcipher_enqueue_givcrypt at the same time as thread two uses ctx->err
to hold the return value of crypto_ablkcipher_encrypt!

Did I make a mistake above?  I suspect more bad things can happen as
well in this scenario, but I'm just focusing on the use of ctx->err here.

 > 
 > Please let me know whether it actually fixes your problem though
 > so I can get this upstream.

Unfortunately, the offset problem is not easily reproduced with our
application, so testing long enough to be sure the problem is fixed
(assuming that it was indeed the cause of the oops) may not be
practical.  All I can say at the moment is that I have not seen the
crash since I introduced the two patches I sent you.

Thanks for taking the time to discuss this!

--Brad
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] crypto: Add VMAC(AES) to kernel for intel_txt support (resend)

2009-09-02 Thread Herbert Xu
On Tue, Sep 01, 2009 at 09:07:15PM +0800, Shane Wang wrote:
>
> I am wondering whether the email system converts tab indent into spaces.
> Please use the attached file (it is the same as the plain text below).
> This patch is based on the latest  
> git://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git

Patch applied.  Thanks a lot Shane!
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html