diff options
| author | Herbert Xu <herbert@gondor.apana.org.au> | 2015-04-07 09:27:01 -0400 |
|---|---|---|
| committer | Herbert Xu <herbert@gondor.apana.org.au> | 2015-04-08 10:20:06 -0400 |
| commit | 016baaa1183bb0c5fb2a7de42413bba8a51c1bc8 (patch) | |
| tree | b6a1705e4cf6a1d504532630fe923ef261f6b2a5 /crypto | |
| parent | 9cd223239a79df3cc758ecabb8473ca91599021b (diff) | |
crypto: user - Fix crypto_alg_match race
The function crypto_alg_match returns an algorithm without taking
any references on it. This means that the algorithm can be freed
at any time, therefore all users of crypto_alg_match are buggy.
This patch fixes this by taking a reference count on the algorithm
to prevent such races.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Diffstat (limited to 'crypto')
| -rw-r--r-- | crypto/crypto_user.c | 39 |
1 files changed, 29 insertions, 10 deletions
diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c index eab249723830..41dfe762b7fb 100644 --- a/crypto/crypto_user.c +++ b/crypto/crypto_user.c | |||
| @@ -62,10 +62,14 @@ static struct crypto_alg *crypto_alg_match(struct crypto_user_alg *p, int exact) | |||
| 62 | else if (!exact) | 62 | else if (!exact) |
| 63 | match = !strcmp(q->cra_name, p->cru_name); | 63 | match = !strcmp(q->cra_name, p->cru_name); |
| 64 | 64 | ||
| 65 | if (match) { | 65 | if (!match) |
| 66 | alg = q; | 66 | continue; |
| 67 | break; | 67 | |
| 68 | } | 68 | if (unlikely(!crypto_mod_get(q))) |
| 69 | continue; | ||
| 70 | |||
| 71 | alg = q; | ||
| 72 | break; | ||
| 69 | } | 73 | } |
| 70 | 74 | ||
| 71 | up_read(&crypto_alg_sem); | 75 | up_read(&crypto_alg_sem); |
| @@ -205,9 +209,10 @@ static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh, | |||
| 205 | if (!alg) | 209 | if (!alg) |
| 206 | return -ENOENT; | 210 | return -ENOENT; |
| 207 | 211 | ||
| 212 | err = -ENOMEM; | ||
| 208 | skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC); | 213 | skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC); |
| 209 | if (!skb) | 214 | if (!skb) |
| 210 | return -ENOMEM; | 215 | goto drop_alg; |
| 211 | 216 | ||
| 212 | info.in_skb = in_skb; | 217 | info.in_skb = in_skb; |
| 213 | info.out_skb = skb; | 218 | info.out_skb = skb; |
| @@ -215,6 +220,10 @@ static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh, | |||
| 215 | info.nlmsg_flags = 0; | 220 | info.nlmsg_flags = 0; |
| 216 | 221 | ||
| 217 | err = crypto_report_alg(alg, &info); | 222 | err = crypto_report_alg(alg, &info); |
| 223 | |||
| 224 | drop_alg: | ||
| 225 | crypto_mod_put(alg); | ||
| 226 | |||
| 218 | if (err) | 227 | if (err) |
| 219 | return err; | 228 | return err; |
| 220 | 229 | ||
| @@ -284,6 +293,7 @@ static int crypto_update_alg(struct sk_buff *skb, struct nlmsghdr *nlh, | |||
| 284 | 293 | ||
| 285 | up_write(&crypto_alg_sem); | 294 | up_write(&crypto_alg_sem); |
| 286 | 295 | ||
| 296 | crypto_mod_put(alg); | ||
| 287 | crypto_remove_final(&list); | 297 | crypto_remove_final(&list); |
| 288 | 298 | ||
| 289 | return 0; | 299 | return 0; |
| @@ -294,6 +304,7 @@ static int crypto_del_alg(struct sk_buff *skb, struct nlmsghdr *nlh, | |||
| 294 | { | 304 | { |
| 295 | struct crypto_alg *alg; | 305 | struct crypto_alg *alg; |
| 296 | struct crypto_user_alg *p = nlmsg_data(nlh); | 306 | struct crypto_user_alg *p = nlmsg_data(nlh); |
| 307 | int err; | ||
| 297 | 308 | ||
| 298 | if (!netlink_capable(skb, CAP_NET_ADMIN)) | 309 | if (!netlink_capable(skb, CAP_NET_ADMIN)) |
| 299 | return -EPERM; | 310 | return -EPERM; |
| @@ -310,13 +321,19 @@ static int crypto_del_alg(struct sk_buff *skb, struct nlmsghdr *nlh, | |||
| 310 | * if we try to unregister. Unregistering such an algorithm without | 321 | * if we try to unregister. Unregistering such an algorithm without |
| 311 | * removing the module is not possible, so we restrict to crypto | 322 | * removing the module is not possible, so we restrict to crypto |
| 312 | * instances that are build from templates. */ | 323 | * instances that are build from templates. */ |
| 324 | err = -EINVAL; | ||
| 313 | if (!(alg->cra_flags & CRYPTO_ALG_INSTANCE)) | 325 | if (!(alg->cra_flags & CRYPTO_ALG_INSTANCE)) |
| 314 | return -EINVAL; | 326 | goto drop_alg; |
| 315 | 327 | ||
| 316 | if (atomic_read(&alg->cra_refcnt) != 1) | 328 | err = -EBUSY; |
| 317 | return -EBUSY; | 329 | if (atomic_read(&alg->cra_refcnt) > 2) |
| 330 | goto drop_alg; | ||
| 318 | 331 | ||
| 319 | return crypto_unregister_instance((struct crypto_instance *)alg); | 332 | err = crypto_unregister_instance((struct crypto_instance *)alg); |
| 333 | |||
| 334 | drop_alg: | ||
| 335 | crypto_mod_put(alg); | ||
| 336 | return err; | ||
| 320 | } | 337 | } |
| 321 | 338 | ||
| 322 | static struct crypto_alg *crypto_user_skcipher_alg(const char *name, u32 type, | 339 | static struct crypto_alg *crypto_user_skcipher_alg(const char *name, u32 type, |
| @@ -395,8 +412,10 @@ static int crypto_add_alg(struct sk_buff *skb, struct nlmsghdr *nlh, | |||
| 395 | return -EINVAL; | 412 | return -EINVAL; |
| 396 | 413 | ||
| 397 | alg = crypto_alg_match(p, exact); | 414 | alg = crypto_alg_match(p, exact); |
| 398 | if (alg) | 415 | if (alg) { |
| 416 | crypto_mod_put(alg); | ||
| 399 | return -EEXIST; | 417 | return -EEXIST; |
| 418 | } | ||
| 400 | 419 | ||
| 401 | if (strlen(p->cru_driver_name)) | 420 | if (strlen(p->cru_driver_name)) |
| 402 | name = p->cru_driver_name; | 421 | name = p->cru_driver_name; |
