All of lore.kernel.org
 help / color / mirror / Atom feed
From: Julian Wiedmann <jwi@linux.ibm.com>
To: David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org, linux-s390@vger.kernel.org,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Stefan Raspl <raspl@linux.ibm.com>,
	Ursula Braun <ubraun@linux.ibm.com>,
	Julian Wiedmann <jwi@linux.ibm.com>
Subject: [PATCH net-next 05/11] s390/qeth: clean up error path in qeth_core_probe_device()
Date: Thu, 14 Nov 2019 11:19:18 +0100	[thread overview]
Message-ID: <20191114101924.29558-6-jwi@linux.ibm.com> (raw)
In-Reply-To: <20191114101924.29558-1-jwi@linux.ibm.com>

qeth_core_free_card() is meant to be the counterpart of
qeth_alloc_card() - but unfortunately was also picked as the place
to free the QDIO queues.

This gets messy when qeth_core_probe_device() fails during
qeth_add_dbf_entry(). At this point the card->qdio.state is not initialized
yet, so qeth_free_qdio_queues() ends up operating on uninitialized data.

Luckily for now, the whole qeth_card struct is zero-allocated and the value
of the QETH_QDIO_UNINITIALIZED enum is 0 as well. So there's no real impact
from this bug at the moment, it's just really fragile.

Clean this up by moving the qeth_free_qdio_queues() call up one level in
the hierarchy. This way it doesn't get called from the error path.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_core_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 08185f76a727..f1f56e354516 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -4855,7 +4855,6 @@ static void qeth_core_free_card(struct qeth_card *card)
 	qeth_clean_channel(&card->data);
 	qeth_put_cmd(card->read_cmd);
 	destroy_workqueue(card->event_wq);
-	qeth_free_qdio_queues(card);
 	unregister_service_level(&card->qeth_service_level);
 	dev_set_drvdata(&card->gdev->dev, NULL);
 	kfree(card);
@@ -5768,6 +5767,8 @@ static void qeth_core_remove_device(struct ccwgroup_device *gdev)
 		qeth_core_free_discipline(card);
 	}
 
+	qeth_free_qdio_queues(card);
+
 	free_netdev(card->dev);
 	qeth_core_free_card(card);
 	put_device(&gdev->dev);
-- 
2.17.1

WARNING: multiple messages have this Message-ID (diff)
From: Julian Wiedmann <jwi@linux.ibm.com>
To: David Miller <davem@davemloft.net>
Cc: <netdev@vger.kernel.org>, <linux-s390@vger.kernel.org>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Stefan Raspl <raspl@linux.ibm.com>,
	Ursula Braun <ubraun@linux.ibm.com>,
	Julian Wiedmann <jwi@linux.ibm.com>
Subject: [PATCH net-next 05/11] s390/qeth: clean up error path in qeth_core_probe_device()
Date: Thu, 14 Nov 2019 11:19:18 +0100	[thread overview]
Message-ID: <20191114101924.29558-6-jwi@linux.ibm.com> (raw)
In-Reply-To: <20191114101924.29558-1-jwi@linux.ibm.com>

qeth_core_free_card() is meant to be the counterpart of
qeth_alloc_card() - but unfortunately was also picked as the place
to free the QDIO queues.

This gets messy when qeth_core_probe_device() fails during
qeth_add_dbf_entry(). At this point the card->qdio.state is not initialized
yet, so qeth_free_qdio_queues() ends up operating on uninitialized data.

Luckily for now, the whole qeth_card struct is zero-allocated and the value
of the QETH_QDIO_UNINITIALIZED enum is 0 as well. So there's no real impact
from this bug at the moment, it's just really fragile.

Clean this up by moving the qeth_free_qdio_queues() call up one level in
the hierarchy. This way it doesn't get called from the error path.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_core_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 08185f76a727..f1f56e354516 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -4855,7 +4855,6 @@ static void qeth_core_free_card(struct qeth_card *card)
 	qeth_clean_channel(&card->data);
 	qeth_put_cmd(card->read_cmd);
 	destroy_workqueue(card->event_wq);
-	qeth_free_qdio_queues(card);
 	unregister_service_level(&card->qeth_service_level);
 	dev_set_drvdata(&card->gdev->dev, NULL);
 	kfree(card);
@@ -5768,6 +5767,8 @@ static void qeth_core_remove_device(struct ccwgroup_device *gdev)
 		qeth_core_free_discipline(card);
 	}
 
+	qeth_free_qdio_queues(card);
+
 	free_netdev(card->dev);
 	qeth_core_free_card(card);
 	put_device(&gdev->dev);
-- 
2.17.1


  parent reply	other threads:[~2019-11-14 10:20 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-14 10:19 [PATCH net-next 00/11] s390/qeth: updates 2019-11-14 Julian Wiedmann
2019-11-14 10:19 ` Julian Wiedmann
2019-11-14 10:19 ` [PATCH net-next 01/11] s390/qeth: gather more detailed RX dropped/error statistics Julian Wiedmann
2019-11-14 10:19   ` Julian Wiedmann
2019-11-14 10:19 ` [PATCH net-next 02/11] s390/qeth: support per-frame invalidation Julian Wiedmann
2019-11-14 10:19   ` Julian Wiedmann
2019-11-14 10:19 ` [PATCH net-next 03/11] s390/qeth: drop unwanted packets earlier in RX path Julian Wiedmann
2019-11-14 10:19   ` Julian Wiedmann
2019-11-14 10:19 ` [PATCH net-next 04/11] s390/qeth: handle skb allocation error gracefully Julian Wiedmann
2019-11-14 10:19   ` Julian Wiedmann
2019-11-14 10:19 ` Julian Wiedmann [this message]
2019-11-14 10:19   ` [PATCH net-next 05/11] s390/qeth: clean up error path in qeth_core_probe_device() Julian Wiedmann
2019-11-14 10:19 ` [PATCH net-next 06/11] s390/qeth: fine-tune L3 mcast locking Julian Wiedmann
2019-11-14 10:19   ` Julian Wiedmann
2019-11-14 10:19 ` [PATCH net-next 07/11] s390/qeth: remove gratuitious RX modeset Julian Wiedmann
2019-11-14 10:19   ` Julian Wiedmann
2019-11-14 10:19 ` [PATCH net-next 08/11] s390/qeth: consolidate L3 mcast registration code Julian Wiedmann
2019-11-14 10:19   ` Julian Wiedmann
2019-11-14 10:19 ` [PATCH net-next 09/11] s390/qeth: remove VLAN tracking for L3 devices Julian Wiedmann
2019-11-14 10:19   ` Julian Wiedmann
2019-11-14 10:19 ` [PATCH net-next 10/11] s390/qeth: replace qeth_l3_get_addr_buffer() Julian Wiedmann
2019-11-14 10:19   ` Julian Wiedmann
2019-11-14 10:19 ` [PATCH net-next 11/11] s390/qeth: don't check drvdata in sysfs code Julian Wiedmann
2019-11-14 10:19   ` Julian Wiedmann
2019-11-15  2:17 ` [PATCH net-next 00/11] s390/qeth: updates 2019-11-14 David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191114101924.29558-6-jwi@linux.ibm.com \
    --to=jwi@linux.ibm.com \
    --cc=davem@davemloft.net \
    --cc=heiko.carstens@de.ibm.com \
    --cc=linux-s390@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=raspl@linux.ibm.com \
    --cc=ubraun@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.