Commit 05b4149433ffae789edaf569da8d998c93eed1aa

Authored by Jan Kiszka
Committed by David S. Miller
1 parent eca39dd830

CAPI: Rework locking of capidev members

Rename 'ncci_list_mtx' to 'lock', expressing that it now protects a
larger set of capidev members: the NCCI list, ap.applid (ie. the
registration of the application), and modifications of userflags.

We do not need to protect each and every check for ap.applid because,
once an application is registered, it will stay for the whole lifetime
of the device.

Also, there is no need to apply the capidev mutex during release (if
there could be concurrent users, we would crash them anyway by freeing
the device at the end of capi_release).

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
Signed-off-by: David S. Miller <davem@davemloft.net>

Showing 1 changed file with 88 additions and 93 deletions Side-by-side Diff

drivers/isdn/capi/capi.c
... ... @@ -141,7 +141,7 @@
141 141  
142 142 struct capincci *nccis;
143 143  
144   - struct mutex ncci_list_mtx;
  144 + struct mutex lock;
145 145 };
146 146  
147 147 /* -------- global variables ---------------------------------------- */
148 148  
149 149  
150 150  
151 151  
152 152  
153 153  
154 154  
155 155  
... ... @@ -574,38 +574,31 @@
574 574 u16 datahandle;
575 575 #endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
576 576 struct capincci *np;
577   - u32 ncci;
578 577 unsigned long flags;
579 578  
  579 + mutex_lock(&cdev->lock);
  580 +
580 581 if (CAPIMSG_CMD(skb->data) == CAPI_CONNECT_B3_CONF) {
581 582 u16 info = CAPIMSG_U16(skb->data, 12); // Info field
582   - if ((info & 0xff00) == 0) {
583   - mutex_lock(&cdev->ncci_list_mtx);
  583 + if ((info & 0xff00) == 0)
584 584 capincci_alloc(cdev, CAPIMSG_NCCI(skb->data));
585   - mutex_unlock(&cdev->ncci_list_mtx);
586   - }
587 585 }
588   - if (CAPIMSG_CMD(skb->data) == CAPI_CONNECT_B3_IND) {
589   - mutex_lock(&cdev->ncci_list_mtx);
  586 + if (CAPIMSG_CMD(skb->data) == CAPI_CONNECT_B3_IND)
590 587 capincci_alloc(cdev, CAPIMSG_NCCI(skb->data));
591   - mutex_unlock(&cdev->ncci_list_mtx);
592   - }
  588 +
593 589 spin_lock_irqsave(&workaround_lock, flags);
594 590 if (CAPIMSG_COMMAND(skb->data) != CAPI_DATA_B3) {
595 591 skb_queue_tail(&cdev->recvqueue, skb);
596 592 wake_up_interruptible(&cdev->recvwait);
597   - spin_unlock_irqrestore(&workaround_lock, flags);
598   - return;
  593 + goto unlock_out;
599 594 }
600   - ncci = CAPIMSG_CONTROL(skb->data);
601   - for (np = cdev->nccis; np && np->ncci != ncci; np = np->next)
602   - ;
  595 +
  596 + np = capincci_find(cdev, CAPIMSG_CONTROL(skb->data));
603 597 if (!np) {
604 598 printk(KERN_ERR "BUG: capi_signal: ncci not found\n");
605 599 skb_queue_tail(&cdev->recvqueue, skb);
606 600 wake_up_interruptible(&cdev->recvwait);
607   - spin_unlock_irqrestore(&workaround_lock, flags);
608   - return;
  601 + goto unlock_out;
609 602 }
610 603  
611 604 #ifndef CONFIG_ISDN_CAPI_MIDDLEWARE
... ... @@ -618,8 +611,7 @@
618 611 if (!mp) {
619 612 skb_queue_tail(&cdev->recvqueue, skb);
620 613 wake_up_interruptible(&cdev->recvwait);
621   - spin_unlock_irqrestore(&workaround_lock, flags);
622   - return;
  614 + goto unlock_out;
623 615 }
624 616 if (CAPIMSG_SUBCOMMAND(skb->data) == CAPI_IND) {
625 617 datahandle = CAPIMSG_U16(skb->data, CAPIMSG_BASELEN+4+4+2);
626 618  
... ... @@ -652,7 +644,9 @@
652 644 }
653 645 #endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
654 646  
  647 +unlock_out:
655 648 spin_unlock_irqrestore(&workaround_lock, flags);
  649 + mutex_unlock(&cdev->lock);
656 650 }
657 651  
658 652 /* -------- file_operations for capidev ----------------------------- */
659 653  
... ... @@ -730,9 +724,9 @@
730 724 CAPIMSG_SETAPPID(skb->data, cdev->ap.applid);
731 725  
732 726 if (CAPIMSG_CMD(skb->data) == CAPI_DISCONNECT_B3_RESP) {
733   - mutex_lock(&cdev->ncci_list_mtx);
  727 + mutex_lock(&cdev->lock);
734 728 capincci_free(cdev, CAPIMSG_NCCI(skb->data));
735   - mutex_unlock(&cdev->ncci_list_mtx);
  729 + mutex_unlock(&cdev->lock);
736 730 }
737 731  
738 732 cdev->errcode = capi20_put_message(&cdev->ap, skb);
739 733  
740 734  
741 735  
742 736  
... ... @@ -765,31 +759,36 @@
765 759 unsigned int cmd, unsigned long arg)
766 760 {
767 761 struct capidev *cdev = file->private_data;
768   - struct capi20_appl *ap = &cdev->ap;
769 762 capi_ioctl_struct data;
770 763 int retval = -EINVAL;
771 764 void __user *argp = (void __user *)arg;
772 765  
773 766 switch (cmd) {
774 767 case CAPI_REGISTER:
775   - {
776   - if (ap->applid)
777   - return -EEXIST;
  768 + mutex_lock(&cdev->lock);
778 769  
779   - if (copy_from_user(&cdev->ap.rparam, argp,
780   - sizeof(struct capi_register_params)))
781   - return -EFAULT;
782   -
783   - cdev->ap.private = cdev;
784   - cdev->ap.recv_message = capi_recv_message;
785   - cdev->errcode = capi20_register(ap);
786   - if (cdev->errcode) {
787   - ap->applid = 0;
788   - return -EIO;
789   - }
  770 + if (cdev->ap.applid) {
  771 + retval = -EEXIST;
  772 + goto register_out;
790 773 }
791   - return (int)ap->applid;
  774 + if (copy_from_user(&cdev->ap.rparam, argp,
  775 + sizeof(struct capi_register_params))) {
  776 + retval = -EFAULT;
  777 + goto register_out;
  778 + }
  779 + cdev->ap.private = cdev;
  780 + cdev->ap.recv_message = capi_recv_message;
  781 + cdev->errcode = capi20_register(&cdev->ap);
  782 + retval = (int)cdev->ap.applid;
  783 + if (cdev->errcode) {
  784 + cdev->ap.applid = 0;
  785 + retval = -EIO;
  786 + }
792 787  
  788 +register_out:
  789 + mutex_unlock(&cdev->lock);
  790 + return retval;
  791 +
793 792 case CAPI_GET_VERSION:
794 793 {
795 794 if (copy_from_user(&data.contr, argp,
796 795  
797 796  
798 797  
799 798  
800 799  
801 800  
802 801  
803 802  
... ... @@ -887,68 +886,67 @@
887 886 return 0;
888 887  
889 888 case CAPI_SET_FLAGS:
890   - case CAPI_CLR_FLAGS:
891   - {
892   - unsigned userflags;
893   - if (copy_from_user(&userflags, argp,
894   - sizeof(userflags)))
895   - return -EFAULT;
896   - if (cmd == CAPI_SET_FLAGS)
897   - cdev->userflags |= userflags;
898   - else
899   - cdev->userflags &= ~userflags;
900   - }
901   - return 0;
  889 + case CAPI_CLR_FLAGS: {
  890 + unsigned userflags;
902 891  
  892 + if (copy_from_user(&userflags, argp, sizeof(userflags)))
  893 + return -EFAULT;
  894 +
  895 + mutex_lock(&cdev->lock);
  896 + if (cmd == CAPI_SET_FLAGS)
  897 + cdev->userflags |= userflags;
  898 + else
  899 + cdev->userflags &= ~userflags;
  900 + mutex_unlock(&cdev->lock);
  901 + return 0;
  902 + }
903 903 case CAPI_GET_FLAGS:
904 904 if (copy_to_user(argp, &cdev->userflags,
905 905 sizeof(cdev->userflags)))
906 906 return -EFAULT;
907 907 return 0;
908 908  
909   - case CAPI_NCCI_OPENCOUNT:
910   - {
911   - struct capincci *nccip;
912   - unsigned ncci;
913   - int count = 0;
914   - if (copy_from_user(&ncci, argp, sizeof(ncci)))
915   - return -EFAULT;
  909 + case CAPI_NCCI_OPENCOUNT: {
  910 + struct capincci *nccip;
  911 + unsigned ncci;
  912 + int count = 0;
916 913  
917   - mutex_lock(&cdev->ncci_list_mtx);
918   - if ((nccip = capincci_find(cdev, (u32) ncci)) == NULL) {
919   - mutex_unlock(&cdev->ncci_list_mtx);
920   - return 0;
921   - }
922   - count += capincci_minor_opencount(nccip);
923   - mutex_unlock(&cdev->ncci_list_mtx);
924   - return count;
925   - }
926   - return 0;
  914 + if (copy_from_user(&ncci, argp, sizeof(ncci)))
  915 + return -EFAULT;
927 916  
  917 + mutex_lock(&cdev->lock);
  918 + nccip = capincci_find(cdev, (u32)ncci);
  919 + if (nccip)
  920 + count = capincci_minor_opencount(nccip);
  921 + mutex_unlock(&cdev->lock);
  922 + return count;
  923 + }
  924 +
928 925 #ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
929   - case CAPI_NCCI_GETUNIT:
930   - {
931   - struct capincci *nccip;
932   - struct capiminor *mp;
933   - unsigned ncci;
934   - int unit = 0;
935   - if (copy_from_user(&ncci, argp,
936   - sizeof(ncci)))
937   - return -EFAULT;
938   - mutex_lock(&cdev->ncci_list_mtx);
939   - nccip = capincci_find(cdev, (u32) ncci);
940   - if (!nccip || (mp = nccip->minorp) == NULL) {
941   - mutex_unlock(&cdev->ncci_list_mtx);
942   - return -ESRCH;
943   - }
944   - unit = mp->minor;
945   - mutex_unlock(&cdev->ncci_list_mtx);
946   - return unit;
  926 + case CAPI_NCCI_GETUNIT: {
  927 + struct capincci *nccip;
  928 + struct capiminor *mp;
  929 + unsigned ncci;
  930 + int unit = -ESRCH;
  931 +
  932 + if (copy_from_user(&ncci, argp, sizeof(ncci)))
  933 + return -EFAULT;
  934 +
  935 + mutex_lock(&cdev->lock);
  936 + nccip = capincci_find(cdev, (u32)ncci);
  937 + if (nccip) {
  938 + mp = nccip->minorp;
  939 + if (mp)
  940 + unit = mp->minor;
947 941 }
948   - return 0;
  942 + mutex_unlock(&cdev->lock);
  943 + return unit;
  944 + }
949 945 #endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
  946 +
  947 + default:
  948 + return -EINVAL;
950 949 }
951   - return -EINVAL;
952 950 }
953 951  
954 952 static int capi_open(struct inode *inode, struct file *file)
... ... @@ -959,7 +957,7 @@
959 957 if (!cdev)
960 958 return -ENOMEM;
961 959  
962   - mutex_init(&cdev->ncci_list_mtx);
  960 + mutex_init(&cdev->lock);
963 961 skb_queue_head_init(&cdev->recvqueue);
964 962 init_waitqueue_head(&cdev->recvwait);
965 963 file->private_data = cdev;
966 964  
967 965  
968 966  
... ... @@ -979,15 +977,10 @@
979 977 list_del(&cdev->list);
980 978 mutex_unlock(&capidev_list_lock);
981 979  
982   - if (cdev->ap.applid) {
  980 + if (cdev->ap.applid)
983 981 capi20_release(&cdev->ap);
984   - cdev->ap.applid = 0;
985   - }
986 982 skb_queue_purge(&cdev->recvqueue);
987   -
988   - mutex_lock(&cdev->ncci_list_mtx);
989 983 capincci_free(cdev, 0xffffffff);
990   - mutex_unlock(&cdev->ncci_list_mtx);
991 984  
992 985 kfree(cdev);
993 986 return 0;
994 987  
... ... @@ -1446,11 +1439,13 @@
1446 1439 mutex_lock(&capidev_list_lock);
1447 1440 list_for_each(l, &capidev_list) {
1448 1441 cdev = list_entry(l, struct capidev, list);
  1442 + mutex_lock(&cdev->lock);
1449 1443 for (np=cdev->nccis; np; np = np->next) {
1450 1444 seq_printf(m, "%d 0x%x\n",
1451 1445 cdev->ap.applid,
1452 1446 np->ncci);
1453 1447 }
  1448 + mutex_unlock(&cdev->lock);
1454 1449 }
1455 1450 mutex_unlock(&capidev_list_lock);
1456 1451 return 0;