linux-kernel - [PATCH 5.8 057/124] xsk: Do not discard packet when NETDEV_TX

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: <20201012133149.619495048@linuxfoundation.org>
Date:   Mon, 12 Oct 2020 15:31:01 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     linux-kernel@...r.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org, Arkadiusz Zema <A.Zema@...convsystems.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        Jesse Brandeburg <jesse.brandeburg@...el.com>,
        Sasha Levin <sashal@...nel.org>
Subject: [PATCH 5.8 057/124] xsk: Do not discard packet when NETDEV_TX_BUSY

From: Magnus Karlsson <magnus.karlsson@...el.com>

[ Upstream commit 642e450b6b5955f2059d0ae372183f7c6323f951 ]

In the skb Tx path, transmission of a packet is performed with
dev_direct_xmit(). When NETDEV_TX_BUSY is set in the drivers, it
signifies that it was not possible to send the packet right now,
please try later. Unfortunately, the xsk transmit code discarded the
packet and returned EBUSY to the application. Fix this unnecessary
packet loss, by not discarding the packet in the Tx ring and return
EAGAIN. As EAGAIN is returned to the application, it can then retry
the send operation later and the packet will then likely be sent as
the driver will then likely have space/resources to send the packet.

In summary, EAGAIN tells the application that the packet was not
discarded from the Tx ring and that it needs to call send()
again. EBUSY, on the other hand, signifies that the packet was not
sent and discarded from the Tx ring. The application needs to put
the packet on the Tx ring again if it wants it to be sent.

Fixes: 35fcde7f8deb ("xsk: support for Tx")
Reported-by: Arkadiusz Zema <A.Zema@...convsystems.com>
Suggested-by: Arkadiusz Zema <A.Zema@...convsystems.com>
Suggested-by: Daniel Borkmann <daniel@...earbox.net>
Signed-off-by: Magnus Karlsson <magnus.karlsson@...el.com>
Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
Reviewed-by: Jesse Brandeburg <jesse.brandeburg@...el.com>
Link: https://lore.kernel.org/bpf/1600257625-2353-1-git-send-email-magnus.karlsson@gmail.com
Signed-off-by: Sasha Levin <sashal@...nel.org>
---
 net/xdp/xsk.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 3700266229f63..dcce888b8ef54 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -375,15 +375,30 @@ static int xsk_generic_xmit(struct sock *sk)
 		skb_shinfo(skb)->destructor_arg = (void *)(long)desc.addr;
 		skb->destructor = xsk_destruct_skb;
 
+		/* Hinder dev_direct_xmit from freeing the packet and
+		 * therefore completing it in the destructor
+		 */
+		refcount_inc(&skb->users);
 		err = dev_direct_xmit(skb, xs->queue_id);
+		if  (err == NETDEV_TX_BUSY) {
+			/* Tell user-space to retry the send */
+			skb->destructor = sock_wfree;
+			/* Free skb without triggering the perf drop trace */
+			consume_skb(skb);
+			err = -EAGAIN;
+			goto out;
+		}
+
 		xskq_cons_release(xs->tx);
 		/* Ignore NET_XMIT_CN as packet might have been sent */
-		if (err == NET_XMIT_DROP || err == NETDEV_TX_BUSY) {
+		if (err == NET_XMIT_DROP) {
 			/* SKB completed but not sent */
+			kfree_skb(skb);
 			err = -EBUSY;
 			goto out;
 		}
 
+		consume_skb(skb);
 		sent_frame = true;
 	}
 
-- 
2.25.1