Commit f70e2e06196ad4c1c762037da2f75354f6c16b81

Authored by David Howells
Committed by James Morris
1 parent 043b4d40f5

KEYS: Do preallocation for __key_link()

Do preallocation for __key_link() so that the various callers in request_key.c
can deal with any errors from this source before attempting to construct a key.
This allows them to assume that the actual linkage step is guaranteed to be
successful.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>

Showing 4 changed files with 215 additions and 130 deletions Side-by-side Diff

security/keys/internal.h
... ... @@ -87,7 +87,16 @@
87 87 extern struct key_type *key_type_lookup(const char *type);
88 88 extern void key_type_put(struct key_type *ktype);
89 89  
90   -extern int __key_link(struct key *keyring, struct key *key);
  90 +extern int __key_link_begin(struct key *keyring,
  91 + const struct key_type *type,
  92 + const char *description,
  93 + struct keyring_list **_prealloc);
  94 +extern int __key_link_check_live_key(struct key *keyring, struct key *key);
  95 +extern void __key_link(struct key *keyring, struct key *key,
  96 + struct keyring_list **_prealloc);
  97 +extern void __key_link_end(struct key *keyring,
  98 + struct key_type *type,
  99 + struct keyring_list *prealloc);
91 100  
92 101 extern key_ref_t __keyring_search_one(key_ref_t keyring_ref,
93 102 const struct key_type *type,
... ... @@ -398,7 +398,8 @@
398 398 const void *data,
399 399 size_t datalen,
400 400 struct key *keyring,
401   - struct key *authkey)
  401 + struct key *authkey,
  402 + struct keyring_list **_prealloc)
402 403 {
403 404 int ret, awaken;
404 405  
... ... @@ -425,7 +426,7 @@
425 426  
426 427 /* and link it into the destination keyring */
427 428 if (keyring)
428   - ret = __key_link(keyring, key);
  429 + __key_link(keyring, key, _prealloc);
429 430  
430 431 /* disable the authorisation key */
431 432 if (authkey)
432 433  
433 434  
434 435  
... ... @@ -453,15 +454,21 @@
453 454 struct key *keyring,
454 455 struct key *authkey)
455 456 {
  457 + struct keyring_list *prealloc;
456 458 int ret;
457 459  
458   - if (keyring)
459   - down_write(&keyring->sem);
  460 + if (keyring) {
  461 + ret = __key_link_begin(keyring, key->type, key->description,
  462 + &prealloc);
  463 + if (ret < 0)
  464 + return ret;
  465 + }
460 466  
461   - ret = __key_instantiate_and_link(key, data, datalen, keyring, authkey);
  467 + ret = __key_instantiate_and_link(key, data, datalen, keyring, authkey,
  468 + &prealloc);
462 469  
463 470 if (keyring)
464   - up_write(&keyring->sem);
  471 + __key_link_end(keyring, key->type, prealloc);
465 472  
466 473 return ret;
467 474  
468 475  
... ... @@ -478,8 +485,9 @@
478 485 struct key *keyring,
479 486 struct key *authkey)
480 487 {
  488 + struct keyring_list *prealloc;
481 489 struct timespec now;
482   - int ret, awaken;
  490 + int ret, awaken, link_ret = 0;
483 491  
484 492 key_check(key);
485 493 key_check(keyring);
... ... @@ -488,7 +496,8 @@
488 496 ret = -EBUSY;
489 497  
490 498 if (keyring)
491   - down_write(&keyring->sem);
  499 + link_ret = __key_link_begin(keyring, key->type,
  500 + key->description, &prealloc);
492 501  
493 502 mutex_lock(&key_construction_mutex);
494 503  
... ... @@ -508,8 +517,8 @@
508 517 ret = 0;
509 518  
510 519 /* and link it into the destination keyring */
511   - if (keyring)
512   - ret = __key_link(keyring, key);
  520 + if (keyring && link_ret == 0)
  521 + __key_link(keyring, key, &prealloc);
513 522  
514 523 /* disable the authorisation key */
515 524 if (authkey)
516 525  
... ... @@ -519,13 +528,13 @@
519 528 mutex_unlock(&key_construction_mutex);
520 529  
521 530 if (keyring)
522   - up_write(&keyring->sem);
  531 + __key_link_end(keyring, key->type, prealloc);
523 532  
524 533 /* wake up anyone waiting for a key to be constructed */
525 534 if (awaken)
526 535 wake_up_bit(&key->flags, KEY_FLAG_USER_CONSTRUCT);
527 536  
528   - return ret;
  537 + return ret == 0 ? link_ret : ret;
529 538  
530 539 } /* end key_negate_and_link() */
531 540  
... ... @@ -749,6 +758,7 @@
749 758 key_perm_t perm,
750 759 unsigned long flags)
751 760 {
  761 + struct keyring_list *prealloc;
752 762 const struct cred *cred = current_cred();
753 763 struct key_type *ktype;
754 764 struct key *keyring, *key = NULL;
... ... @@ -775,7 +785,9 @@
775 785 if (keyring->type != &key_type_keyring)
776 786 goto error_2;
777 787  
778   - down_write(&keyring->sem);
  788 + ret = __key_link_begin(keyring, ktype, description, &prealloc);
  789 + if (ret < 0)
  790 + goto error_2;
779 791  
780 792 /* if we're going to allocate a new key, we're going to have
781 793 * to modify the keyring */
... ... @@ -817,7 +829,8 @@
817 829 }
818 830  
819 831 /* instantiate it and link it into the target keyring */
820   - ret = __key_instantiate_and_link(key, payload, plen, keyring, NULL);
  832 + ret = __key_instantiate_and_link(key, payload, plen, keyring, NULL,
  833 + &prealloc);
821 834 if (ret < 0) {
822 835 key_put(key);
823 836 key_ref = ERR_PTR(ret);
... ... @@ -827,7 +840,7 @@
827 840 key_ref = make_key_ref(key, is_key_possessed(keyring_ref));
828 841  
829 842 error_3:
830   - up_write(&keyring->sem);
  843 + __key_link_end(keyring, ktype, prealloc);
831 844 error_2:
832 845 key_type_put(ktype);
833 846 error:
... ... @@ -837,7 +850,7 @@
837 850 /* we found a matching key, so we're going to try to update it
838 851 * - we can drop the locks first as we have the key pinned
839 852 */
840   - up_write(&keyring->sem);
  853 + __key_link_end(keyring, ktype, prealloc);
841 854 key_type_put(ktype);
842 855  
843 856 key_ref = __key_update(key_ref, payload, plen);
security/keys/keyring.c
... ... @@ -660,21 +660,7 @@
660 660  
661 661 } /* end keyring_detect_cycle() */
662 662  
663   -/*****************************************************************************/
664 663 /*
665   - * dispose of a keyring list after the RCU grace period
666   - */
667   -static void keyring_link_rcu_disposal(struct rcu_head *rcu)
668   -{
669   - struct keyring_list *klist =
670   - container_of(rcu, struct keyring_list, rcu);
671   -
672   - kfree(klist);
673   -
674   -} /* end keyring_link_rcu_disposal() */
675   -
676   -/*****************************************************************************/
677   -/*
678 664 * dispose of a keyring list after the RCU grace period, freeing the unlinked
679 665 * key
680 666 */
681 667  
682 668  
683 669  
684 670  
685 671  
686 672  
687 673  
688 674  
689 675  
690 676  
691 677  
692 678  
693 679  
... ... @@ -683,56 +669,51 @@
683 669 struct keyring_list *klist =
684 670 container_of(rcu, struct keyring_list, rcu);
685 671  
686   - key_put(klist->keys[klist->delkey]);
  672 + if (klist->delkey != USHORT_MAX)
  673 + key_put(klist->keys[klist->delkey]);
687 674 kfree(klist);
  675 +}
688 676  
689   -} /* end keyring_unlink_rcu_disposal() */
690   -
691   -/*****************************************************************************/
692 677 /*
693   - * link a key into to a keyring
694   - * - must be called with the keyring's semaphore write-locked
695   - * - discard already extant link to matching key if there is one
  678 + * preallocate memory so that a key can be linked into to a keyring
696 679 */
697   -int __key_link(struct key *keyring, struct key *key)
  680 +int __key_link_begin(struct key *keyring, const struct key_type *type,
  681 + const char *description,
  682 + struct keyring_list **_prealloc)
  683 + __acquires(&keyring->sem)
698 684 {
699 685 struct keyring_list *klist, *nklist;
700 686 unsigned max;
701 687 size_t size;
702 688 int loop, ret;
703 689  
704   - ret = -EKEYREVOKED;
705   - if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))
706   - goto error;
  690 + kenter("%d,%s,%s,", key_serial(keyring), type->name, description);
707 691  
708   - ret = -ENOTDIR;
709 692 if (keyring->type != &key_type_keyring)
710   - goto error;
  693 + return -ENOTDIR;
711 694  
712   - /* do some special keyring->keyring link checks */
713   - if (key->type == &key_type_keyring) {
714   - /* serialise link/link calls to prevent parallel calls causing
715   - * a cycle when applied to two keyring in opposite orders */
  695 + down_write(&keyring->sem);
  696 +
  697 + ret = -EKEYREVOKED;
  698 + if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))
  699 + goto error_krsem;
  700 +
  701 + /* serialise link/link calls to prevent parallel calls causing a cycle
  702 + * when linking two keyring in opposite orders */
  703 + if (type == &key_type_keyring)
716 704 down_write(&keyring_serialise_link_sem);
717 705  
718   - /* check that we aren't going to create a cycle adding one
719   - * keyring to another */
720   - ret = keyring_detect_cycle(keyring, key);
721   - if (ret < 0)
722   - goto error2;
723   - }
  706 + klist = rcu_dereference_locked_keyring(keyring);
724 707  
725 708 /* see if there's a matching key we can displace */
726   - klist = rcu_dereference_locked_keyring(keyring);
727 709 if (klist && klist->nkeys > 0) {
728   - struct key_type *type = key->type;
729   -
730 710 for (loop = klist->nkeys - 1; loop >= 0; loop--) {
731 711 if (klist->keys[loop]->type == type &&
732 712 strcmp(klist->keys[loop]->description,
733   - key->description) == 0
  713 + description) == 0
734 714 ) {
735   - /* found a match - replace with new key */
  715 + /* found a match - we'll replace this one with
  716 + * the new key */
736 717 size = sizeof(struct key *) * klist->maxkeys;
737 718 size += sizeof(*klist);
738 719 BUG_ON(size > PAGE_SIZE);
739 720  
... ... @@ -740,22 +721,10 @@
740 721 ret = -ENOMEM;
741 722 nklist = kmemdup(klist, size, GFP_KERNEL);
742 723 if (!nklist)
743   - goto error2;
  724 + goto error_sem;
744 725  
745   - /* replace matched key */
746   - atomic_inc(&key->usage);
747   - nklist->keys[loop] = key;
748   -
749   - rcu_assign_pointer(
750   - keyring->payload.subscriptions,
751   - nklist);
752   -
753   - /* dispose of the old keyring list and the
754   - * displaced key */
755   - klist->delkey = loop;
756   - call_rcu(&klist->rcu,
757   - keyring_unlink_rcu_disposal);
758   -
  726 + /* note replacement slot */
  727 + klist->delkey = nklist->delkey = loop;
759 728 goto done;
760 729 }
761 730 }
762 731  
... ... @@ -765,16 +734,11 @@
765 734 ret = key_payload_reserve(keyring,
766 735 keyring->datalen + KEYQUOTA_LINK_BYTES);
767 736 if (ret < 0)
768   - goto error2;
  737 + goto error_sem;
769 738  
770 739 if (klist && klist->nkeys < klist->maxkeys) {
771   - /* there's sufficient slack space to add directly */
772   - atomic_inc(&key->usage);
773   -
774   - klist->keys[klist->nkeys] = key;
775   - smp_wmb();
776   - klist->nkeys++;
777   - smp_wmb();
  740 + /* there's sufficient slack space to append directly */
  741 + nklist = NULL;
778 742 } else {
779 743 /* grow the key list */
780 744 max = 4;
781 745  
782 746  
783 747  
784 748  
785 749  
786 750  
787 751  
788 752  
789 753  
790 754  
791 755  
792 756  
793 757  
794 758  
795 759  
... ... @@ -782,71 +746,155 @@
782 746 max += klist->maxkeys;
783 747  
784 748 ret = -ENFILE;
785   - if (max > 65535)
786   - goto error3;
  749 + if (max > USHORT_MAX - 1)
  750 + goto error_quota;
787 751 size = sizeof(*klist) + sizeof(struct key *) * max;
788 752 if (size > PAGE_SIZE)
789   - goto error3;
  753 + goto error_quota;
790 754  
791 755 ret = -ENOMEM;
792 756 nklist = kmalloc(size, GFP_KERNEL);
793 757 if (!nklist)
794   - goto error3;
795   - nklist->maxkeys = max;
796   - nklist->nkeys = 0;
  758 + goto error_quota;
797 759  
  760 + nklist->maxkeys = max;
798 761 if (klist) {
799   - nklist->nkeys = klist->nkeys;
800   - memcpy(nklist->keys,
801   - klist->keys,
  762 + memcpy(nklist->keys, klist->keys,
802 763 sizeof(struct key *) * klist->nkeys);
  764 + nklist->delkey = klist->nkeys;
  765 + nklist->nkeys = klist->nkeys + 1;
  766 + klist->delkey = USHORT_MAX;
  767 + } else {
  768 + nklist->nkeys = 1;
  769 + nklist->delkey = 0;
803 770 }
804 771  
805 772 /* add the key into the new space */
806   - atomic_inc(&key->usage);
807   - nklist->keys[nklist->nkeys++] = key;
808   -
809   - rcu_assign_pointer(keyring->payload.subscriptions, nklist);
810   -
811   - /* dispose of the old keyring list */
812   - if (klist)
813   - call_rcu(&klist->rcu, keyring_link_rcu_disposal);
  773 + nklist->keys[nklist->delkey] = NULL;
814 774 }
815 775  
816 776 done:
817   - ret = 0;
818   -error2:
819   - if (key->type == &key_type_keyring)
820   - up_write(&keyring_serialise_link_sem);
821   -error:
822   - return ret;
  777 + *_prealloc = nklist;
  778 + kleave(" = 0");
  779 + return 0;
823 780  
824   -error3:
  781 +error_quota:
825 782 /* undo the quota changes */
826 783 key_payload_reserve(keyring,
827 784 keyring->datalen - KEYQUOTA_LINK_BYTES);
828   - goto error2;
  785 +error_sem:
  786 + if (type == &key_type_keyring)
  787 + up_write(&keyring_serialise_link_sem);
  788 +error_krsem:
  789 + up_write(&keyring->sem);
  790 + kleave(" = %d", ret);
  791 + return ret;
  792 +}
829 793  
830   -} /* end __key_link() */
  794 +/*
  795 + * check already instantiated keys aren't going to be a problem
  796 + * - the caller must have called __key_link_begin()
  797 + * - don't need to call this for keys that were created since __key_link_begin()
  798 + * was called
  799 + */
  800 +int __key_link_check_live_key(struct key *keyring, struct key *key)
  801 +{
  802 + if (key->type == &key_type_keyring)
  803 + /* check that we aren't going to create a cycle by linking one
  804 + * keyring to another */
  805 + return keyring_detect_cycle(keyring, key);
  806 + return 0;
  807 +}
831 808  
832   -/*****************************************************************************/
833 809 /*
  810 + * link a key into to a keyring
  811 + * - must be called with __key_link_begin() having being called
  812 + * - discard already extant link to matching key if there is one
  813 + */
  814 +void __key_link(struct key *keyring, struct key *key,
  815 + struct keyring_list **_prealloc)
  816 +{
  817 + struct keyring_list *klist, *nklist;
  818 +
  819 + nklist = *_prealloc;
  820 + *_prealloc = NULL;
  821 +
  822 + kenter("%d,%d,%p", keyring->serial, key->serial, nklist);
  823 +
  824 + klist = rcu_dereference_protected(keyring->payload.subscriptions,
  825 + rwsem_is_locked(&keyring->sem));
  826 +
  827 + atomic_inc(&key->usage);
  828 +
  829 + /* there's a matching key we can displace or an empty slot in a newly
  830 + * allocated list we can fill */
  831 + if (nklist) {
  832 + kdebug("replace %hu/%hu/%hu",
  833 + nklist->delkey, nklist->nkeys, nklist->maxkeys);
  834 +
  835 + nklist->keys[nklist->delkey] = key;
  836 +
  837 + rcu_assign_pointer(keyring->payload.subscriptions, nklist);
  838 +
  839 + /* dispose of the old keyring list and, if there was one, the
  840 + * displaced key */
  841 + if (klist) {
  842 + kdebug("dispose %hu/%hu/%hu",
  843 + klist->delkey, klist->nkeys, klist->maxkeys);
  844 + call_rcu(&klist->rcu, keyring_unlink_rcu_disposal);
  845 + }
  846 + } else {
  847 + /* there's sufficient slack space to append directly */
  848 + klist->keys[klist->nkeys] = key;
  849 + smp_wmb();
  850 + klist->nkeys++;
  851 + }
  852 +}
  853 +
  854 +/*
  855 + * finish linking a key into to a keyring
  856 + * - must be called with __key_link_begin() having being called
  857 + */
  858 +void __key_link_end(struct key *keyring, struct key_type *type,
  859 + struct keyring_list *prealloc)
  860 + __releases(&keyring->sem)
  861 +{
  862 + BUG_ON(type == NULL);
  863 + BUG_ON(type->name == NULL);
  864 + kenter("%d,%s,%p", keyring->serial, type->name, prealloc);
  865 +
  866 + if (type == &key_type_keyring)
  867 + up_write(&keyring_serialise_link_sem);
  868 +
  869 + if (prealloc) {
  870 + kfree(prealloc);
  871 + key_payload_reserve(keyring,
  872 + keyring->datalen - KEYQUOTA_LINK_BYTES);
  873 + }
  874 + up_write(&keyring->sem);
  875 +}
  876 +
  877 +/*
834 878 * link a key to a keyring
835 879 */
836 880 int key_link(struct key *keyring, struct key *key)
837 881 {
  882 + struct keyring_list *prealloc;
838 883 int ret;
839 884  
840 885 key_check(keyring);
841 886 key_check(key);
842 887  
843   - down_write(&keyring->sem);
844   - ret = __key_link(keyring, key);
845   - up_write(&keyring->sem);
  888 + ret = __key_link_begin(keyring, key->type, key->description, &prealloc);
  889 + if (ret == 0) {
  890 + ret = __key_link_check_live_key(keyring, key);
  891 + if (ret == 0)
  892 + __key_link(keyring, key, &prealloc);
  893 + __key_link_end(keyring, key->type, prealloc);
  894 + }
846 895  
847 896 return ret;
848   -
849   -} /* end key_link() */
  897 +}
850 898  
851 899 EXPORT_SYMBOL(key_link);
852 900  
security/keys/request_key.c
... ... @@ -299,6 +299,7 @@
299 299 struct key_user *user,
300 300 struct key **_key)
301 301 {
  302 + struct keyring_list *prealloc;
302 303 const struct cred *cred = current_cred();
303 304 struct key *key;
304 305 key_ref_t key_ref;
... ... @@ -306,6 +307,7 @@
306 307  
307 308 kenter("%s,%s,,,", type->name, description);
308 309  
  310 + *_key = NULL;
309 311 mutex_lock(&user->cons_lock);
310 312  
311 313 key = key_alloc(type, description, cred->fsuid, cred->fsgid, cred,
... ... @@ -315,8 +317,12 @@
315 317  
316 318 set_bit(KEY_FLAG_USER_CONSTRUCT, &key->flags);
317 319  
318   - if (dest_keyring)
319   - down_write(&dest_keyring->sem);
  320 + if (dest_keyring) {
  321 + ret = __key_link_begin(dest_keyring, type, description,
  322 + &prealloc);
  323 + if (ret < 0)
  324 + goto link_prealloc_failed;
  325 + }
320 326  
321 327 /* attach the key to the destination keyring under lock, but we do need
322 328 * to do another check just in case someone beat us to it whilst we
323 329  
... ... @@ -328,11 +334,11 @@
328 334 goto key_already_present;
329 335  
330 336 if (dest_keyring)
331   - __key_link(dest_keyring, key);
  337 + __key_link(dest_keyring, key, &prealloc);
332 338  
333 339 mutex_unlock(&key_construction_mutex);
334 340 if (dest_keyring)
335   - up_write(&dest_keyring->sem);
  341 + __key_link_end(dest_keyring, type, prealloc);
336 342 mutex_unlock(&user->cons_lock);
337 343 *_key = key;
338 344 kleave(" = 0 [%d]", key_serial(key));
339 345  
340 346  
341 347  
342 348  
343 349  
... ... @@ -341,27 +347,36 @@
341 347 /* the key is now present - we tell the caller that we found it by
342 348 * returning -EINPROGRESS */
343 349 key_already_present:
  350 + key_put(key);
344 351 mutex_unlock(&key_construction_mutex);
345   - ret = 0;
  352 + key = key_ref_to_ptr(key_ref);
346 353 if (dest_keyring) {
347   - ret = __key_link(dest_keyring, key_ref_to_ptr(key_ref));
348   - up_write(&dest_keyring->sem);
  354 + ret = __key_link_check_live_key(dest_keyring, key);
  355 + if (ret == 0)
  356 + __key_link(dest_keyring, key, &prealloc);
  357 + __key_link_end(dest_keyring, type, prealloc);
  358 + if (ret < 0)
  359 + goto link_check_failed;
349 360 }
350 361 mutex_unlock(&user->cons_lock);
351   - key_put(key);
352   - if (ret < 0) {
353   - key_ref_put(key_ref);
354   - *_key = NULL;
355   - kleave(" = %d [link]", ret);
356   - return ret;
357   - }
358   - *_key = key = key_ref_to_ptr(key_ref);
  362 + *_key = key;
359 363 kleave(" = -EINPROGRESS [%d]", key_serial(key));
360 364 return -EINPROGRESS;
361 365  
  366 +link_check_failed:
  367 + mutex_unlock(&user->cons_lock);
  368 + key_put(key);
  369 + kleave(" = %d [linkcheck]", ret);
  370 + return ret;
  371 +
  372 +link_prealloc_failed:
  373 + up_write(&dest_keyring->sem);
  374 + mutex_unlock(&user->cons_lock);
  375 + kleave(" = %d [prelink]", ret);
  376 + return ret;
  377 +
362 378 alloc_failed:
363 379 mutex_unlock(&user->cons_lock);
364   - *_key = NULL;
365 380 kleave(" = %ld", PTR_ERR(key));
366 381 return PTR_ERR(key);
367 382 }