s390/qeth: fix locking for discipline setup / removal
authorJulian Wiedmann <jwi@linux.ibm.com>
Thu, 7 Jan 2021 17:24:41 +0000 (18:24 +0100)
committerJakub Kicinski <kuba@kernel.org>
Fri, 8 Jan 2021 02:54:06 +0000 (18:54 -0800)
Due to insufficient locking, qeth_core_set_online() and
qeth_dev_layer2_store() can run in parallel, both attempting to load &
setup the discipline (and stepping on each other toes along the way).
A similar race can also occur between qeth_core_remove_device() and
qeth_dev_layer2_store().

Access to .discipline is meant to be protected by the discipline_mutex,
so add/expand the locking in qeth_core_remove_device() and
qeth_core_set_online().
Adjust the locking in qeth_l*_remove_device() accordingly, as it's now
handled by the callers in a consistent manner.

Based on an initial patch by Ursula Braun.

Fixes: 9dc48ccc68b9 ("qeth: serialize sysfs-triggered device configurations")
Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
drivers/s390/net/qeth_core_main.c
drivers/s390/net/qeth_l2_main.c
drivers/s390/net/qeth_l3_main.c

index d45e223fc521878be698d6cb5631ab575f788494..cf18d87da41e21cbd9e8fbd4703d3c8ed658ad21 100644 (file)
@@ -6585,6 +6585,7 @@ static int qeth_core_probe_device(struct ccwgroup_device *gdev)
                break;
        default:
                card->info.layer_enforced = true;
+               /* It's so early that we don't need the discipline_mutex yet. */
                rc = qeth_core_load_discipline(card, enforced_disc);
                if (rc)
                        goto err_load;
@@ -6617,10 +6618,12 @@ static void qeth_core_remove_device(struct ccwgroup_device *gdev)
 
        QETH_CARD_TEXT(card, 2, "removedv");
 
+       mutex_lock(&card->discipline_mutex);
        if (card->discipline) {
                card->discipline->remove(gdev);
                qeth_core_free_discipline(card);
        }
+       mutex_unlock(&card->discipline_mutex);
 
        qeth_free_qdio_queues(card);
 
@@ -6635,6 +6638,7 @@ static int qeth_core_set_online(struct ccwgroup_device *gdev)
        int rc = 0;
        enum qeth_discipline_id def_discipline;
 
+       mutex_lock(&card->discipline_mutex);
        if (!card->discipline) {
                def_discipline = IS_IQD(card) ? QETH_DISCIPLINE_LAYER3 :
                                                QETH_DISCIPLINE_LAYER2;
@@ -6648,11 +6652,10 @@ static int qeth_core_set_online(struct ccwgroup_device *gdev)
                }
        }
 
-       mutex_lock(&card->discipline_mutex);
        rc = qeth_set_online(card, card->discipline);
-       mutex_unlock(&card->discipline_mutex);
 
 err:
+       mutex_unlock(&card->discipline_mutex);
        return rc;
 }
 
index 37279b1e29f6ef6443dc96767a7f1327f74f5115..4254caf1d9b699e062b2e98a1afdcd1eeec579e3 100644 (file)
@@ -2207,11 +2207,8 @@ static void qeth_l2_remove_device(struct ccwgroup_device *gdev)
        qeth_set_allowed_threads(card, 0, 1);
        wait_event(card->wait_q, qeth_threads_running(card, 0xffffffff) == 0);
 
-       if (gdev->state == CCWGROUP_ONLINE) {
-               mutex_lock(&card->discipline_mutex);
+       if (gdev->state == CCWGROUP_ONLINE)
                qeth_set_offline(card, card->discipline, false);
-               mutex_unlock(&card->discipline_mutex);
-       }
 
        cancel_work_sync(&card->close_dev_work);
        if (card->dev->reg_state == NETREG_REGISTERED)
index 8d474179ce982506724c5f973d8cb5e50bd5a9c6..6970597bc8859342b0934ebfaa3249051ade2518 100644 (file)
@@ -1970,11 +1970,8 @@ static void qeth_l3_remove_device(struct ccwgroup_device *cgdev)
        qeth_set_allowed_threads(card, 0, 1);
        wait_event(card->wait_q, qeth_threads_running(card, 0xffffffff) == 0);
 
-       if (cgdev->state == CCWGROUP_ONLINE) {
-               mutex_lock(&card->discipline_mutex);
+       if (cgdev->state == CCWGROUP_ONLINE)
                qeth_set_offline(card, card->discipline, false);
-               mutex_unlock(&card->discipline_mutex);
-       }
 
        cancel_work_sync(&card->close_dev_work);
        if (card->dev->reg_state == NETREG_REGISTERED)