On Thu, Nov 20, 2014 at 05:23:23AM +0100, Stephan Mueller wrote:
>
> Here is the code:
>
> static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
> struct nlattr **attrs)
> {
> ...
> if (!p->cru_driver_name[0])
> return -EINVAL;
OK let's fix this.
crypto: user - Allow get request with empty driver name
Currently all get requests with an empty driver name fail with
EINVAL. Since most users actually want to supply an empty driver
name this patch removes this check.
Signed-off-by: Herbert Xu <[email protected]>
diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index e2a34fe..0bb30ac 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -201,10 +201,7 @@ static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
if (!null_terminated(p->cru_name) || !null_terminated(p->cru_driver_name))
return -EINVAL;
- if (!p->cru_driver_name[0])
- return -EINVAL;
-
- alg = crypto_alg_match(p, 1);
+ alg = crypto_alg_match(p, 0);
if (!alg)
return -ENOENT;
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Thu, Nov 20, 2014 at 12:46:50PM +0800, Herbert Xu wrote:
> On Thu, Nov 20, 2014 at 05:23:23AM +0100, Stephan Mueller wrote:
> >
> > Here is the code:
> >
> > static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
> > struct nlattr **attrs)
> > {
> > ...
> > if (!p->cru_driver_name[0])
> > return -EINVAL;
>
> OK let's fix this.
>
> crypto: user - Allow get request with empty driver name
>
> Currently all get requests with an empty driver name fail with
> EINVAL. Since most users actually want to supply an empty driver
> name this patch removes this check.
>
> Signed-off-by: Herbert Xu <[email protected]>
>
> diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
> index e2a34fe..0bb30ac 100644
> --- a/crypto/crypto_user.c
> +++ b/crypto/crypto_user.c
> @@ -201,10 +201,7 @@ static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
> if (!null_terminated(p->cru_name) || !null_terminated(p->cru_driver_name))
> return -EINVAL;
>
> - if (!p->cru_driver_name[0])
> - return -EINVAL;
> -
> - alg = crypto_alg_match(p, 1);
> + alg = crypto_alg_match(p, 0);
I think this is not sufficient, crypto_alg_match() will now return the first
algorithm in crypto_alg_list that matches cra_name. We would need to extend
crypto_alg_match() to return the algorithm with the highest priority in that
case.
On Thu, Nov 20, 2014 at 08:11:42AM +0100, Steffen Klassert wrote:
>
> I think this is not sufficient, crypto_alg_match() will now return the first
> algorithm in crypto_alg_list that matches cra_name. We would need to extend
> crypto_alg_match() to return the algorithm with the highest priority in that
> case.
It doesn't really matter because all implementations must provide
exactly the same set of parameters for a given algorithm so it's
good enough for what Stephan wants to do.
But yes an interface to grab the highest priority algorithm would
be useful too so patches are welcome :)
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Thu, Nov 20, 2014 at 03:45:26PM +0800, Herbert Xu wrote:
> On Thu, Nov 20, 2014 at 08:11:42AM +0100, Steffen Klassert wrote:
> >
> > I think this is not sufficient, crypto_alg_match() will now return the first
> > algorithm in crypto_alg_list that matches cra_name. We would need to extend
> > crypto_alg_match() to return the algorithm with the highest priority in that
> > case.
>
> It doesn't really matter because all implementations must provide
> exactly the same set of parameters for a given algorithm so it's
> good enough for what Stephan wants to do.
Ok, I see.
> But yes an interface to grab the highest priority algorithm would
> be useful too so patches are welcome :)
Should be not too hard to return the highest priority algorithm
instead of the first match with the existing interface, I'll see
what I can do.
Am Donnerstag, 20. November 2014, 12:46:50 schrieb Herbert Xu:
Hi Herbert,
> On Thu, Nov 20, 2014 at 05:23:23AM +0100, Stephan Mueller wrote:
> > Here is the code:
> >
> > static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
> >
> > struct nlattr **attrs)
> >
> > {
> > ...
> >
> > if (!p->cru_driver_name[0])
> >
> > return -EINVAL;
>
> OK let's fix this.
>
> crypto: user - Allow get request with empty driver name
>
> Currently all get requests with an empty driver name fail with
> EINVAL. Since most users actually want to supply an empty driver
> name this patch removes this check.
>
> Signed-off-by: Herbert Xu <[email protected]>
Acked-by: Stephan Mueller <[email protected]>
>
> diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
> index e2a34fe..0bb30ac 100644
> --- a/crypto/crypto_user.c
> +++ b/crypto/crypto_user.c
> @@ -201,10 +201,7 @@ static int crypto_report(struct sk_buff *in_skb, struct
> nlmsghdr *in_nlh, if (!null_terminated(p->cru_name) ||
> !null_terminated(p->cru_driver_name)) return -EINVAL;
>
> - if (!p->cru_driver_name[0])
> - return -EINVAL;
> -
> - alg = crypto_alg_match(p, 1);
> + alg = crypto_alg_match(p, 0);
> if (!alg)
> return -ENOENT;
>
> Cheers,
--
Ciao
Stephan
Am Donnerstag, 20. November 2014, 09:04:06 schrieb Steffen Klassert:
Hi Steffen,
> On Thu, Nov 20, 2014 at 03:45:26PM +0800, Herbert Xu wrote:
> > On Thu, Nov 20, 2014 at 08:11:42AM +0100, Steffen Klassert wrote:
> > > I think this is not sufficient, crypto_alg_match() will now return the
> > > first algorithm in crypto_alg_list that matches cra_name. We would need
> > > to extend crypto_alg_match() to return the algorithm with the highest
> > > priority in that case.
> >
> > It doesn't really matter because all implementations must provide
> > exactly the same set of parameters for a given algorithm so it's
> > good enough for what Stephan wants to do.
>
> Ok, I see.
>
> > But yes an interface to grab the highest priority algorithm would
> > be useful too so patches are welcome :)
>
> Should be not too hard to return the highest priority algorithm
> instead of the first match with the existing interface, I'll see
> what I can do.
I think that for the purpose of using crypto-user in an AF_ALG scenario, even
searching for the highest priporty will not necessarily give you the reference
to the cipher used in AF_ALG. In the time between AF_ALG initialized a cipher
and the time crypto-user is called, a new driver could register that may have
even a higher priority than the one driver currently active for AF_ALG.
Therefore, from my perspective of AF_ALG and considering that all drivers of
the same name should be identical, I would not suggest add more code to
resolve the highest priority as I do not see the value of it.
--
Ciao
Stephan
Am Donnerstag, 20. November 2014, 14:02:21 schrieb Stephan Mueller:
Hi Stephan,
> Am Donnerstag, 20. November 2014, 12:46:50 schrieb Herbert Xu:
>
> Hi Herbert,
>
> > On Thu, Nov 20, 2014 at 05:23:23AM +0100, Stephan Mueller wrote:
> > > Here is the code:
> > >
> > > static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr
> > > *in_nlh,
> > >
> > > struct nlattr **attrs)
> > >
> > > {
> > > ...
> > >
> > > if (!p->cru_driver_name[0])
> > >
> > > return -EINVAL;
> >
> > OK let's fix this.
> >
> > crypto: user - Allow get request with empty driver name
> >
> > Currently all get requests with an empty driver name fail with
> > EINVAL. Since most users actually want to supply an empty driver
> > name this patch removes this check.
> >
> > Signed-off-by: Herbert Xu <[email protected]>
>
> Acked-by: Stephan Mueller <[email protected]>
Although I acked the patch, it just occurred that your change modifies the
user space interface such that you now cannot use a driver name any more.
>
> > diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
> > index e2a34fe..0bb30ac 100644
> > --- a/crypto/crypto_user.c
> > +++ b/crypto/crypto_user.c
> > @@ -201,10 +201,7 @@ static int crypto_report(struct sk_buff *in_skb,
> > struct nlmsghdr *in_nlh, if (!null_terminated(p->cru_name) ||
> > !null_terminated(p->cru_driver_name)) return -EINVAL;
> >
> > - if (!p->cru_driver_name[0])
> > - return -EINVAL;
> > -
> > - alg = crypto_alg_match(p, 1);
> > + alg = crypto_alg_match(p, 0);
What about the following
if (p->cru_driver_name[0]
alg = crypto_alg_match(p, 1);
else
alg = crypto_alg_match(p, 0);
> >
> > if (!alg)
> >
> > return -ENOENT;
> >
> > Cheers,
--
Ciao
Stephan
On Thu, Nov 20, 2014 at 02:10:00PM +0100, Stephan Mueller wrote:
>
> What about the following
>
> if (p->cru_driver_name[0]
> alg = crypto_alg_match(p, 1);
> else
> alg = crypto_alg_match(p, 0);
If cru_driver_name is not empty then exact will never be used, no?
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 Donnerstag, 20. November 2014, 21:40:29 schrieb Herbert Xu:
Hi Herbert,
> On Thu, Nov 20, 2014 at 02:10:00PM +0100, Stephan Mueller wrote:
> > What about the following
> >
> > if (p->cru_driver_name[0]
If the driver name is not empty
> >
> > alg = crypto_alg_match(p, 1);
Do an exact match using the driver name
> >
> > else
> >
> > alg = crypto_alg_match(p, 0);
Otherise use the generic name for a fuzzy match.
>
> If cru_driver_name is not empty then exact will never be used, no?
If the code suggestion does not follow my words outlined above, I want my
words to be considered ;-)
Yet, I am unsure where the code deviates from my words.
>
> Cheers,
--
Ciao
Stephan
On Thu, Nov 20, 2014 at 05:08:42PM +0100, Stephan Mueller wrote:
> Am Donnerstag, 20. November 2014, 21:40:29 schrieb Herbert Xu:
>
> Hi Herbert,
>
> > On Thu, Nov 20, 2014 at 02:10:00PM +0100, Stephan Mueller wrote:
> > > What about the following
> > >
> > > if (p->cru_driver_name[0]
>
> If the driver name is not empty
> > >
> > > alg = crypto_alg_match(p, 1);
>
> Do an exact match using the driver name
> > >
> > > else
> > >
> > > alg = crypto_alg_match(p, 0);
>
> Otherise use the generic name for a fuzzy match.
> >
> > If cru_driver_name is not empty then exact will never be used, no?
>
> If the code suggestion does not follow my words outlined above, I want my
> words to be considered ;-)
>
> Yet, I am unsure where the code deviates from my words.
No I am asking how is this different from my patch?
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 Freitag, 21. November 2014, 10:31:31 schrieb Herbert Xu:
Hi Herbert,
> On Thu, Nov 20, 2014 at 05:08:42PM +0100, Stephan Mueller wrote:
> > Am Donnerstag, 20. November 2014, 21:40:29 schrieb Herbert Xu:
> >
> > Hi Herbert,
> >
> > > On Thu, Nov 20, 2014 at 02:10:00PM +0100, Stephan Mueller wrote:
> > > > What about the following
> > > >
> > > > if (p->cru_driver_name[0]
> >
> > If the driver name is not empty
> >
> > > > alg = crypto_alg_match(p, 1);
> >
> > Do an exact match using the driver name
> >
> > > > else
> > > >
> > > > alg = crypto_alg_match(p, 0);
> >
> > Otherise use the generic name for a fuzzy match.
> >
> > > If cru_driver_name is not empty then exact will never be used, no?
> >
> > If the code suggestion does not follow my words outlined above, I want my
> > words to be considered ;-)
> >
> > Yet, I am unsure where the code deviates from my words.
>
> No I am asking how is this different from my patch?
Sorry, I withdraw my comment. Your patch is good as is after rechecking
crypto_alg_match.
>
> Cheers,
--
Ciao
Stephan
Am Donnerstag, 20. November 2014, 14:02:21 schrieb Stephan Mueller:
Hi Stephan,
> Am Donnerstag, 20. November 2014, 12:46:50 schrieb Herbert Xu:
>
> Hi Herbert,
>
> > On Thu, Nov 20, 2014 at 05:23:23AM +0100, Stephan Mueller wrote:
> > > Here is the code:
> > >
> > > static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr
> > > *in_nlh,
> > >
> > > struct nlattr **attrs)
> > >
> > > {
> > > ...
> > >
> > > if (!p->cru_driver_name[0])
> > >
> > > return -EINVAL;
> >
> > OK let's fix this.
> >
> > crypto: user - Allow get request with empty driver name
> >
> > Currently all get requests with an empty driver name fail with
> > EINVAL. Since most users actually want to supply an empty driver
> > name this patch removes this check.
> >
> > Signed-off-by: Herbert Xu <[email protected]>
>
> Acked-by: Stephan Mueller <[email protected]>
>
> > diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
> > index e2a34fe..0bb30ac 100644
> > --- a/crypto/crypto_user.c
> > +++ b/crypto/crypto_user.c
> > @@ -201,10 +201,7 @@ static int crypto_report(struct sk_buff *in_skb,
> > struct nlmsghdr *in_nlh, if (!null_terminated(p->cru_name) ||
> > !null_terminated(p->cru_driver_name)) return -EINVAL;
> >
> > - if (!p->cru_driver_name[0])
> > - return -EINVAL;
> > -
> > - alg = crypto_alg_match(p, 1);
> > + alg = crypto_alg_match(p, 0);
Btw: I tested that patch and it works as expected.
> >
> > if (!alg)
> >
> > return -ENOENT;
> >
> > Cheers,
--
Ciao
Stephan