Commit ca467005c452ed79b90f92419a6c77c3c515de2a

Authored by Bart Van Assche
Committed by Greg Kroah-Hartman
1 parent 91439e675c

IB/umad: Fix error handling

commit 8ec0a0e6b58218bdc1db91dd70ebfcd6ad8dd6cd upstream.

Avoid leaking a kref count in ib_umad_open() if port->ib_dev == NULL
or if nonseekable_open() fails.

Avoid leaking a kref count, that sm_sem is kept down and also that the
IB_PORT_SM capability mask is not cleared in ib_umad_sm_open() if
nonseekable_open() fails.

Since container_of() never returns NULL, remove the code that tests
whether container_of() returns NULL.

Moving the kref_get() call from the start of ib_umad_*open() to the
end is safe since it is the responsibility of the caller of these
functions to ensure that the cdev pointer remains valid until at least
when these functions return.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>

[ydroneaud@opteya.com: rework a bit to reduce the amount of code changed]

Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>

[ nonseekable_open() can't actually fail, but....  - Roland ]

Signed-off-by: Roland Dreier <roland@purestorage.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Showing 1 changed file with 27 additions and 22 deletions Side-by-side Diff

drivers/infiniband/core/user_mad.c
... ... @@ -780,27 +780,19 @@
780 780 {
781 781 struct ib_umad_port *port;
782 782 struct ib_umad_file *file;
783   - int ret;
  783 + int ret = -ENXIO;
784 784  
785 785 port = container_of(inode->i_cdev, struct ib_umad_port, cdev);
786   - if (port)
787   - kref_get(&port->umad_dev->ref);
788   - else
789   - return -ENXIO;
790 786  
791 787 mutex_lock(&port->file_mutex);
792 788  
793   - if (!port->ib_dev) {
794   - ret = -ENXIO;
  789 + if (!port->ib_dev)
795 790 goto out;
796   - }
797 791  
  792 + ret = -ENOMEM;
798 793 file = kzalloc(sizeof *file, GFP_KERNEL);
799   - if (!file) {
800   - kref_put(&port->umad_dev->ref, ib_umad_release_dev);
801   - ret = -ENOMEM;
  794 + if (!file)
802 795 goto out;
803   - }
804 796  
805 797 mutex_init(&file->mutex);
806 798 spin_lock_init(&file->send_lock);
807 799  
... ... @@ -814,7 +806,14 @@
814 806 list_add_tail(&file->port_list, &port->file_list);
815 807  
816 808 ret = nonseekable_open(inode, filp);
  809 + if (ret) {
  810 + list_del(&file->port_list);
  811 + kfree(file);
  812 + goto out;
  813 + }
817 814  
  815 + kref_get(&port->umad_dev->ref);
  816 +
818 817 out:
819 818 mutex_unlock(&port->file_mutex);
820 819 return ret;
... ... @@ -880,10 +879,6 @@
880 879 int ret;
881 880  
882 881 port = container_of(inode->i_cdev, struct ib_umad_port, sm_cdev);
883   - if (port)
884   - kref_get(&port->umad_dev->ref);
885   - else
886   - return -ENXIO;
887 882  
888 883 if (filp->f_flags & O_NONBLOCK) {
889 884 if (down_trylock(&port->sm_sem)) {
890 885  
891 886  
892 887  
... ... @@ -898,17 +893,27 @@
898 893 }
899 894  
900 895 ret = ib_modify_port(port->ib_dev, port->port_num, 0, &props);
901   - if (ret) {
902   - up(&port->sm_sem);
903   - goto fail;
904   - }
  896 + if (ret)
  897 + goto err_up_sem;
905 898  
906 899 filp->private_data = port;
907 900  
908   - return nonseekable_open(inode, filp);
  901 + ret = nonseekable_open(inode, filp);
  902 + if (ret)
  903 + goto err_clr_sm_cap;
909 904  
  905 + kref_get(&port->umad_dev->ref);
  906 +
  907 + return 0;
  908 +
  909 +err_clr_sm_cap:
  910 + swap(props.set_port_cap_mask, props.clr_port_cap_mask);
  911 + ib_modify_port(port->ib_dev, port->port_num, 0, &props);
  912 +
  913 +err_up_sem:
  914 + up(&port->sm_sem);
  915 +
910 916 fail:
911   - kref_put(&port->umad_dev->ref, ib_umad_release_dev);
912 917 return ret;
913 918 }
914 919