[PATCH]crypto: Fix algorithm and driver duplicate registered

[Youquan,Song]

Current kernel, All the algorithms with same algorithm name and driver name
are registered.
But the proper behaviour should be only one algorithm be registered when
algorithm and driver name duplicate.

Signed-off-by: Youquan, Song <youqua...@intel.com>
---

diff --git a/crypto/algapi.c b/crypto/algapi.c
index f149b1c..a823fb2 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -207,8 +207,8 @@ static struct crypto_larval *__crypto_register_alg(struct crypto_alg *alg)
continue;
}

- if (!strcmp(q->cra_driver_name, alg->cra_name) ||
- !strcmp(q->cra_name, alg->cra_driver_name))
+ if (!strcmp(q->cra_driver_name, alg->cra_driver_name) &&
+ !strcmp(q->cra_name, alg->cra_name))
goto err;
}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Herbert Xu]

On Wed, Nov 18, 2009 at 07:32:02PM -0500, Youquan,Song wrote:
> Current kernel, All the algorithms with same algorithm name and driver name
> are registered.
> But the proper behaviour should be only one algorithm be registered when
> algorithm and driver name duplicate.
>
> Signed-off-by: Youquan, Song <youqua...@intel.com>

This is intentional, in order to allow older implementations to be
replaced on the fly.

Is this causing any issues for you?

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <her...@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[Youquan,Song]

On Thu, Nov 19, 2009 at 08:18:27AM +0800, Herbert Xu wrote:
> On Wed, Nov 18, 2009 at 07:32:02PM -0500, Youquan,Song wrote:
> > Current kernel, All the algorithms with same algorithm name and driver name
> > are registered.
> > But the proper behaviour should be only one algorithm be registered when
> > algorithm and driver name duplicate.
> >
> > Signed-off-by: Youquan, Song <youqua...@intel.com>
>
> This is intentional, in order to allow older implementations to be
> replaced on the fly.
>
> Is this causing any issues for you?

I have watched crypto_alg_list includes several duplicate algorithms
with same algorithm name and driver name.
Since they are the same algorithms, one is only needed. otherwise,it
will make some confusion.

Thanks.

[Herbert Xu]

On Thu, Nov 19, 2009 at 06:02:22AM -0500, Youquan,Song wrote:
>
> I have watched crypto_alg_list includes several duplicate algorithms
> with same algorithm name and driver name.
> Since they are the same algorithms, one is only needed. otherwise,it
> will make some confusion.

Can you show me a sample /proc/crypto showing this problem and how
you created it?

Thanks,

--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <her...@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[Youquan,Song]

> Can you show me a sample /proc/crypto showing this problem and how
> you created it?
>

Sorry for late. I use the cryptodev-2.6 tree, enable AESNI and PCLMULQDQ NI.

Step: like this.
A. modprobe tcrypt mode=35
B. modprobe aesni-intel
C. modprobe tcrypt mode=35

After I do A. Because it do not load aesni-intel, so I do B, then repeat C.

cat /proc/crypto.

there is two identical gcm:

name : gcm(aes)
driver : gcm_base(ctr-aes-aesni,ghash-clmulni)
module : gcm
priority : 400
refcnt : 1
selftest : unknown
type : aead
async : yes
blocksize : 1
ivsize : 16
maxauthsize : 16
geniv : <built-in>

name : gcm(aes)
driver : gcm_base(ctr-aes-aesni,ghash-clmulni)
module : gcm
priority : 400
refcnt : 1
selftest : unknown
type : aead
async : yes
blocksize : 1
ivsize : 16
maxauthsize : 16
geniv : <built-in>

name : fpu(ctr(__aes-aesni))
driver : cryptd(fpu(ctr(__driver-aes-aesni)))
module : cryptd
priority : 50
refcnt : 1
selftest : passed
type : ablkcipher
async : yes
blocksize : 1
min keysize : 16
max keysize : 32
ivsize : 16
geniv : <default>

name : fpu(ctr(__aes-aesni))
driver : fpu(ctr(__driver-aes-aesni))
module : fpu
priority : 0
refcnt : 1
selftest : passed
type : blkcipher
blocksize : 1
min keysize : 16
max keysize : 32
ivsize : 16

[Youquan,Song]

On Thu, Nov 19, 2009 at 07:47:30PM +0800, Herbert Xu wrote:
> On Thu, Nov 19, 2009 at 06:02:22AM -0500, Youquan,Song wrote:
> >
> > I have watched crypto_alg_list includes several duplicate algorithms
> > with same algorithm name and driver name.
> > Since they are the same algorithms, one is only needed. otherwise,it
> > will make some confusion.
>

Hi Herbert,

The alg->cra_name and alg->cra_driver_name, from the description,
cra_name is the algorithm name and cra_driver_name is the driver name.
But when I read the code, I often get confuse about these two names.
They often mix each other.

Can you give me some instruction about them? or Can we make them more clear? Thanks.

-Youquan

[Herbert Xu]

On Mon, Nov 23, 2009 at 12:21:17PM -0500, Youquan,Song wrote:
>
> Sorry for late. I use the cryptodev-2.6 tree, enable AESNI and PCLMULQDQ NI.
>
> Step: like this.
> A. modprobe tcrypt mode=35
> B. modprobe aesni-intel
> C. modprobe tcrypt mode=35
>
> After I do A. Because it do not load aesni-intel, so I do B, then repeat C.
>
> cat /proc/crypto.
>
> there is two identical gcm:
>
> name : gcm(aes)
> driver : gcm_base(ctr-aes-aesni,ghash-clmulni)
> module : gcm
> priority : 400
> refcnt : 1
> selftest : unknown

It's creating a second copy because the first one failed the
self-test.

So you should try to identify the reason for the failed self-test.

Cheers,

--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <her...@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[Herbert Xu]

On Mon, Nov 23, 2009 at 12:47:24PM -0500, Youquan,Song wrote:
>
> The alg->cra_name and alg->cra_driver_name, from the description,
> cra_name is the algorithm name and cra_driver_name is the driver name.

For each algorithm you may have an arbitrary number of drivers
implementing it. The driver name must be unique. However,
duplicates are allowed so that a new version of a driver may
be loaded while the older instances of it are still in use.

> But when I read the code, I often get confuse about these two names.
> They often mix each other.
>
> Can you give me some instruction about them? or Can we make them more clear? Thanks.

Well can you tell me what exactly confuses you?

Cheers,

--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <her...@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[Youquan,Song]

> On Mon, Nov 23, 2009 at 12:47:24PM -0500, Youquan,Song wrote:
> >
> > The alg->cra_name and alg->cra_driver_name, from the description,
> > cra_name is the algorithm name and cra_driver_name is the driver name.
>
> For each algorithm you may have an arbitrary number of drivers
> implementing it. The driver name must be unique. However,
> duplicates are allowed so that a new version of a driver may
> be loaded while the older instances of it are still in use.
>
> > But when I read the code, I often get confuse about these two names.
> > They often mix each other.
> >
> > Can you give me some instruction about them? or Can we make them more clear? Thanks.
>
> Well can you tell me what exactly confuses you?

Thanks.
For example:

struct crypto_alg *crypto_larval_lookup(const char *name, u32 type, u32
mask)

The parameter "name", sometime it is "alg->cra_name" while sometime it
become "alg->cra_driver_name". What's to lookup, algorithm or driver, depends on
the context of parameter, So it need confirm no duplicate name exists between
cra_driver_name and cra_name. Therefore, there are some werid checking needed,
such as following:

if (!strcmp(q->cra_driver_name, alg->cra_name) ||

!strcmp(q->cra_name, alg->cra_driver_name))

I wonder, Can we define two functions: one lookup algorithm other lookup
driver? It will be more clear.

Thanks.

[Herbert Xu]

On Wed, Nov 25, 2009 at 05:35:16AM -0500, Youquan,Song wrote:
>
> For example:
>
> struct crypto_alg *crypto_larval_lookup(const char *name, u32 type, u32
> mask)
>
> The parameter "name", sometime it is "alg->cra_name" while sometime it
> become "alg->cra_driver_name". What's to lookup, algorithm or driver, depends on
> the context of parameter, So it need confirm no duplicate name exists between
> cra_driver_name and cra_name. Therefore, there are some werid checking needed,
> such as following:
>
> if (!strcmp(q->cra_driver_name, alg->cra_name) ||
> !strcmp(q->cra_name, alg->cra_driver_name))
>
>
> I wonder, Can we define two functions: one lookup algorithm other lookup
> driver? It will be more clear.

No that would defeat the whole point of having algorithm names
vs. driver names. When you perform a lookup, we will look for
both algorithm matches AND driver matches. Exactly which one
takes precedence when two matches are present is determined by
the priority.

Anyway, none of this matters to you if you're just working with
driver implementations so don't even worry about this.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <her...@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt