| Message ID | 20141120044650.GA28691@gondor.apana.org.au (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
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 <herbert@gondor.apana.org.au> > > 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. -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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,
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. -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <herbert@gondor.apana.org.au> Acked-by: Stephan Mueller <smueller@chronox.de> > > 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,
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.
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 <herbert@gondor.apana.org.au> > > Acked-by: Stephan Mueller <smueller@chronox.de> 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,
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,
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,
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,
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,
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 <herbert@gondor.apana.org.au> > > Acked-by: Stephan Mueller <smueller@chronox.de> > > > 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,
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;