lkml.org 
[lkml]   [2012]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] In crypto_add_alg(), 'exact' wants to be initialized to 0
On Wed, 1 Feb 2012, devendra.aaru wrote:

> On Sun, Jan 29, 2012 at 5:39 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> > We declare 'exact' without initializing it and then do:
> >
> > [...]
> >        if (strlen(p->cru_driver_name))
> >                exact = 1;
> >
> >        if (priority && !exact)
> >                return -EINVAL;
> >
> > [...]
> >
> > If the first 'if' is not true, then the second will test an
> > uninitialized 'exact'.
>
> not needed . as the cru_driver_name will always be present :).

If that is indeed the case, and we are guaranteed that, then it would seem
that a patch like the following would be what we want instead??

Please note that this patch is intended just for discussion, nothing else
(which is why I left out a Signed-off-by on purpose), since I've not
tested it beyond checking that it compiles, nor have I verified your claim
that cru_driver_name will always be present.

Herbert, David, any input?


Subject: [PATCH] Simplify crypto_add_alg() and crypto_alg_match()

Since cru_driver_name will always be present, the test

if (strlen(p->cru_driver_name))
exact = 1;

will always be true. So there's no need to have the test at all, we
can just unconditionally assign 1 to 'exact'.

And if 'exact' is always 1, then we'll never take the true branch of

if (priority && !exact)
return -EINVAL;

so we may as well just remove those two lines completely.

At this point we may as well just remove 'exact' entirely and just use
a hardcoded 1 in the call to crypto_alg_match().

The 'name' variable is also entirely redundant since with
cru_driver_name always being present we'll always take the first
branch in

if (strlen(p->cru_driver_name))
name = p->cru_driver_name;
else
name = p->cru_name;

and then we may as well just use 'p->cru_driver_name' itself the one
place where it is needed in the call to crypto_alg_mod_lookup().

At this point all calls to the static crypto_alg crypto_alg_match()
function pass 1 as the second argument, so there's not really any
point any more in that function having that argument, so let's remove
it and the code that tests it.

And since strlen(p->cru_driver_name) will also always be true in that
function, we can simplify the assignment to 'match'.

---
crypto/crypto_user.c | 33 ++++++++-------------------------
1 files changed, 8 insertions(+), 25 deletions(-)

diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index 16f8693..7760c22 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -38,23 +38,19 @@ struct crypto_dump_info {
u16 nlmsg_flags;
};

-static struct crypto_alg *crypto_alg_match(struct crypto_user_alg *p, int exact)
+static struct crypto_alg *crypto_alg_match(struct crypto_user_alg *p)
{
struct crypto_alg *q, *alg = NULL;

down_read(&crypto_alg_sem);

list_for_each_entry(q, &crypto_alg_list, cra_list) {
- int match = 0;
+ int match;

if ((q->cra_flags ^ p->cru_type) & p->cru_mask)
continue;

- if (strlen(p->cru_driver_name))
- match = !strcmp(q->cra_driver_name,
- p->cru_driver_name);
- else if (!exact)
- match = !strcmp(q->cra_name, p->cru_name);
+ match = !strcmp(q->cra_driver_name, p->cru_driver_name);

if (match) {
alg = q;
@@ -195,7 +191,7 @@ static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
if (!p->cru_driver_name)
return -EINVAL;

- alg = crypto_alg_match(p, 1);
+ alg = crypto_alg_match(p);
if (!alg)
return -ENOENT;

@@ -259,7 +255,7 @@ static int crypto_update_alg(struct sk_buff *skb, struct nlmsghdr *nlh,
if (priority && !strlen(p->cru_driver_name))
return -EINVAL;

- alg = crypto_alg_match(p, 1);
+ alg = crypto_alg_match(p);
if (!alg)
return -ENOENT;

@@ -283,7 +279,7 @@ static int crypto_del_alg(struct sk_buff *skb, struct nlmsghdr *nlh,
struct crypto_alg *alg;
struct crypto_user_alg *p = nlmsg_data(nlh);

- alg = crypto_alg_match(p, 1);
+ alg = crypto_alg_match(p);
if (!alg)
return -ENOENT;

@@ -304,28 +300,15 @@ static int crypto_del_alg(struct sk_buff *skb, struct nlmsghdr *nlh,
static int crypto_add_alg(struct sk_buff *skb, struct nlmsghdr *nlh,
struct nlattr **attrs)
{
- int exact;
- const char *name;
struct crypto_alg *alg;
struct crypto_user_alg *p = nlmsg_data(nlh);
struct nlattr *priority = attrs[CRYPTOCFGA_PRIORITY_VAL];

- if (strlen(p->cru_driver_name))
- exact = 1;
-
- if (priority && !exact)
- return -EINVAL;
-
- alg = crypto_alg_match(p, exact);
+ alg = crypto_alg_match(p);
if (alg)
return -EEXIST;

- if (strlen(p->cru_driver_name))
- name = p->cru_driver_name;
- else
- name = p->cru_name;
-
- alg = crypto_alg_mod_lookup(name, p->cru_type, p->cru_mask);
+ alg = crypto_alg_mod_lookup(p->cru_driver_name, p->cru_type, p->cru_mask);
if (IS_ERR(alg))
return PTR_ERR(alg);

--
1.7.9

--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
\
 
 \ /
  Last update: 2012-02-01 21:23    [W:0.546 / U:0.452 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site