netdev - [PATCH net-next 1/2] s390/qdio: add tighter controls for IRQ polling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20200306081311.50635-2-jwi@linux.ibm.com>
Date:   Fri,  6 Mar 2020 09:13:10 +0100
From:   Julian Wiedmann <jwi@...ux.ibm.com>
To:     David Miller <davem@...emloft.net>
Cc:     netdev <netdev@...r.kernel.org>,
        linux-s390 <linux-s390@...r.kernel.org>,
        Heiko Carstens <heiko.carstens@...ibm.com>,
        Ursula Braun <ubraun@...ux.ibm.com>,
        Julian Wiedmann <jwi@...ux.ibm.com>, Qian Cai <cai@....pw>
Subject: [PATCH net-next 1/2] s390/qdio: add tighter controls for IRQ polling

Once the call to qdio_establish() has completed, qdio is free to deliver
data IRQs to the device driver's IRQ poll handler.

For qeth (the only qdio driver that currently uses IRQ polling) this is
problematic, since the IRQs can arrive before its NAPI instance is
even registered. Calling napi_schedule() from qeth_qdio_start_poll()
then crashes in various nasty ways.

Until recently qeth checked for IFF_UP to drop such early interrupts,
but that's fragile as well since it doesn't enforce any ordering.

Fix this properly by bringing up the qdio device in IRQS_DISABLED mode,
and have the driver explicitly opt-in to receive data IRQs.
qeth does so from qeth_open(), which kick-starts a NAPI poll and then
calls qdio_start_irq() from qeth_poll().

Also add a matching qdio_stop_irq() in qeth_stop() to switch the qdio
dataplane back into a disabled state.

Fixes: 3d35dbe6224e ("s390/qeth: don't check for IFF_UP when scheduling napi")
CC: Qian Cai <cai@....pw>
Reported-by: Qian Cai <cai@....pw>
Signed-off-by: Julian Wiedmann <jwi@...ux.ibm.com>
Acked-by: Vasily Gorbik <gor@...ux.ibm.com>
---
 drivers/s390/cio/qdio_setup.c     | 11 +++++++++--
 drivers/s390/net/qeth_core_main.c |  5 ++---
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/s390/cio/qdio_setup.c b/drivers/s390/cio/qdio_setup.c
index e115623b86b2..66e4bdca9d89 100644
--- a/drivers/s390/cio/qdio_setup.c
+++ b/drivers/s390/cio/qdio_setup.c
@@ -224,8 +224,15 @@ static void setup_queues(struct qdio_irq *irq_ptr,
 		setup_queues_misc(q, irq_ptr, qdio_init->input_handler, i);
 
 		q->is_input_q = 1;
-		q->u.in.queue_start_poll = qdio_init->queue_start_poll_array ?
-				qdio_init->queue_start_poll_array[i] : NULL;
+		if (qdio_init->queue_start_poll_array &&
+		    qdio_init->queue_start_poll_array[i]) {
+			q->u.in.queue_start_poll =
+				qdio_init->queue_start_poll_array[i];
+			set_bit(QDIO_QUEUE_IRQS_DISABLED,
+				&q->u.in.queue_irq_state);
+		} else {
+			q->u.in.queue_start_poll = NULL;
+		}
 
 		setup_storage_lists(q, irq_ptr, input_sbal_array, i);
 		input_sbal_array += QDIO_MAX_BUFFERS_PER_Q;
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index fdc50543ce9a..37c17ad8ee25 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -6582,9 +6582,6 @@ int qeth_open(struct net_device *dev)
 
 	QETH_CARD_TEXT(card, 4, "qethopen");
 
-	if (qdio_stop_irq(CARD_DDEV(card), 0) < 0)
-		return -EIO;
-
 	card->data.state = CH_STATE_UP;
 	netif_tx_start_all_queues(dev);
 
@@ -6634,6 +6631,8 @@ int qeth_stop(struct net_device *dev)
 	}
 
 	napi_disable(&card->napi);
+	qdio_stop_irq(CARD_DDEV(card), 0);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(qeth_stop);
-- 
2.17.1