Hi,
is the core crypto code supposed to "kill" algorithms which fail the
test?
On little-endian IXP4xx 3 hardware-assisted algorithms fail (due to
apparently unrelated bug which I will take care of). It seems the kernel
is still using these failing algorithms (my debugging code adds extra
fields to the /proc output):
alg: skcipher: Test 1 failed on encryption for ecb(des)-ixp4xx
00000000: 01 23 45 67 89 ab cd e7
alg: skcipher: Test 1 failed on encryption for ecb(des3_ede)-ixp4xx
00000000: 73 6f 6d 65 64 61 74 61
alg: skcipher: Test 1 failed on encryption for ecb(aes)-ixp4xx
00000000: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff
# grep 'ecb(des)-ixp4xx\|ecb(des3_ede)-ixp4xx\|ecb(aes)-ixp4xx' /proc/cryp
to -A 6
driver : ecb(aes)-ixp4xx
module : ixp4xx_crypto
priority : 300
refcnt : 1
flags : 0x85
ptr : 0xbf020074
selftest : unknown
^^^^^^^^^^^^^^^^^^^^^^
--
driver : ecb(des3_ede)-ixp4xx
module : ixp4xx_crypto
priority : 300
refcnt : 1
flags : 0x85
ptr : 0xbf01fe94
selftest : unknown
--
driver : ecb(des)-ixp4xx
module : ixp4xx_crypto
priority : 300
refcnt : 1
flags : 0x85
ptr : 0xbf01fcb4
selftest : unknown
I traced the problem to crypto/algapi.c:
int crypto_register_alg(struct crypto_alg *alg)
{
struct crypto_larval *larval;
int err;
err = crypto_check_alg(alg);
if (err)
return err;
down_write(&crypto_alg_sem);
larval = __crypto_register_alg(alg);
up_write(&crypto_alg_sem);
if (IS_ERR(larval))
return PTR_ERR(larval);
crypto_wait_for_test(larval);
At this point alg->cra_flags includes CRYPTO_ALG_DEAD (due to failed
test), but larval->alg.cra_flags has only the original flags (0x85).
I'm not sure what's the best fix.
Currently 2.6.31.9, seems to be present in 2.6.32, too.
--
Krzysztof Halasa
> int crypto_register_alg(struct crypto_alg *alg)
> {
> struct crypto_larval *larval;
> int err;
>
> err = crypto_check_alg(alg);
> if (err)
> return err;
>
> down_write(&crypto_alg_sem);
> larval = __crypto_register_alg(alg);
> up_write(&crypto_alg_sem);
>
> if (IS_ERR(larval))
> return PTR_ERR(larval);
>
> crypto_wait_for_test(larval);
>
> At this point alg->cra_flags includes CRYPTO_ALG_DEAD (due to failed
> test), but larval->alg.cra_flags has only the original flags (0x85).
Actually it seems all alg->cra_flags are CRYPTO_ALG_DEAD at this point,
not only these which failed tests. Will look at it soon.
--
Krzysztof Halasa
Krzysztof Halasa <[email protected]> writes:
> is the core crypto code supposed to "kill" algorithms which fail the
> test?
>
> On little-endian IXP4xx 3 hardware-assisted algorithms fail (due to
> apparently unrelated bug which I will take care of). It seems the kernel
> is still using these failing algorithms (my debugging code adds extra
> fields to the /proc output):
>
> alg: skcipher: Test 1 failed on encryption for ecb(des)-ixp4xx
> 00000000: 01 23 45 67 89 ab cd e7
> alg: skcipher: Test 1 failed on encryption for ecb(des3_ede)-ixp4xx
> 00000000: 73 6f 6d 65 64 61 74 61
> alg: skcipher: Test 1 failed on encryption for ecb(aes)-ixp4xx
> 00000000: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff
>
> # grep 'ecb(des)-ixp4xx\|ecb(des3_ede)-ixp4xx\|ecb(aes)-ixp4xx' /proc/cryp
> to -A 6
> driver : ecb(aes)-ixp4xx
> module : ixp4xx_crypto
> priority : 300
> refcnt : 1
> flags : 0x85
> ptr : 0xbf020074
> selftest : unknown
> ^^^^^^^^^^^^^^^^^^^^^^
I think probably crypto_alg_tested() should mark the failing algorithm
somehow?
void crypto_alg_tested(const char *name, int err)
{
...
found:
q->cra_flags |= CRYPTO_ALG_DEAD;
alg = test->adult;
>>>>>>>>>>>>> maybe here? if (err) mark_as_failed(alg); <<<<<<<<<<<<<<<<
if (err || list_empty(&alg->cra_list))
goto complete;
alg->cra_flags |= CRYPTO_ALG_TESTED;
--
Krzysztof Halasa
Krzysztof Halasa <[email protected]> wrote:
>
> On little-endian IXP4xx 3 hardware-assisted algorithms fail (due to
> apparently unrelated bug which I will take care of). It seems the kernel
> is still using these failing algorithms (my debugging code adds extra
> fields to the /proc output):
How did you determine that it was still being used? When a kernel
user requests for an algorithm the system is supposed to skip
anything which failed the self-test.
> At this point alg->cra_flags includes CRYPTO_ALG_DEAD (due to failed
> test), but larval->alg.cra_flags has only the original flags (0x85).
CRYPTO_ALG_DEAD is used to mark algorithms deleted from the
system. However, we don't delete algorithms just because they
fail the self-test. They remain in the system so you can come
back and diagnose the problem. They just aren't used by anyone.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Herbert Xu <[email protected]> writes:
>> On little-endian IXP4xx 3 hardware-assisted algorithms fail (due to
>> apparently unrelated bug which I will take care of). It seems the kernel
>> is still using these failing algorithms (my debugging code adds extra
>> fields to the /proc output):
>
> How did you determine that it was still being used? When a kernel
> user requests for an algorithm the system is supposed to skip
> anything which failed the self-test.
cat /proc/crypto shows "selftest: unknown" for those failed tests. I
don't know if that means it's used, but I'd expect "failed" or something
like that. Maybe it's simply a problem in /proc/crypto output.
> CRYPTO_ALG_DEAD is used to mark algorithms deleted from the
> system. However, we don't delete algorithms just because they
> fail the self-test. They remain in the system so you can come
> back and diagnose the problem. They just aren't used by anyone.
Great.
Currently the /proc/crypto contains:
- for passed tests: "selftest: passed" (which is of course right)
- for failed tests: "selftest: unknown" (which is a surprise for me):
alg: skcipher: Test 1 failed on encryption for ecb(des)-ixp4xx
00000000: 01 23 45 67 89 ab cd e7
name : ecb(des)
driver : ecb(des)-ixp4xx
module : ixp4xx_crypto
priority : 300
refcnt : 1
selftest : unknown
type : ablkcipher
async : yes
blocksize : 8
min keysize : 8
max keysize : 8
ivsize : 0
geniv : <default>
- for routines without a test: "selftest: passed" (which isn't true
either)
alg: No test for authenc(hmac(md5),cbc(des)) (authenc(hmac(md5),cbc(des))-ixp4xx)
name : authenc(hmac(md5),cbc(des))
driver : authenc(hmac(md5),cbc(des))-ixp4xx
module : ixp4xx_crypto
priority : 300
refcnt : 1
selftest : passed
type : aead
async : yes
blocksize : 8
ivsize : 8
maxauthsize : 16
geniv : <built-in>
I think we need a way to differentiate between "really unknown" and
"failed", do we need another flag for it?
--
Krzysztof Halasa
On Tue, Jan 12, 2010 at 06:55:07PM +0100, Krzysztof Halasa wrote:
>
> Currently the /proc/crypto contains:
>
> - for passed tests: "selftest: passed" (which is of course right)
>
> - for failed tests: "selftest: unknown" (which is a surprise for me):
>
> alg: skcipher: Test 1 failed on encryption for ecb(des)-ixp4xx
> 00000000: 01 23 45 67 89 ab cd e7
>
> name : ecb(des)
> driver : ecb(des)-ixp4xx
> module : ixp4xx_crypto
> priority : 300
> refcnt : 1
> selftest : unknown
> type : ablkcipher
> async : yes
> blocksize : 8
> min keysize : 8
> max keysize : 8
> ivsize : 0
> geniv : <default>
This is fine.
> - for routines without a test: "selftest: passed" (which isn't true
> either)
>
> alg: No test for authenc(hmac(md5),cbc(des)) (authenc(hmac(md5),cbc(des))-ixp4xx)
This should be fixed by adding some code to test authenc algorithms
by combining the test vectors of its components.
> I think we need a way to differentiate between "really unknown" and
> "failed", do we need another flag for it?
It doesn't matter. We test all algorithms at registration time.
So unknown means failed except during registration.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Herbert Xu <[email protected]> writes:
>> I think we need a way to differentiate between "really unknown" and
>> "failed", do we need another flag for it?
>
> It doesn't matter. We test all algorithms at registration time.
> So unknown means failed except during registration.
So should we change "selftest: unknown" to "failed" then?
--
Krzysztof Halasa