[PATCH net-next v2 4/9] tun: correct drop statistics in tun_get_user

[Jon Kohler]

Improve on commit 4b4f052e2d89 ("net: tun: track dropped skb via
kfree_skb_reason()") and commit ab00af85d2f8 ("net: tun: rebuild error
handling in tun_get_user") by updating all potential drop sites in
tun_get_user with appropriate drop reasons.

Rework goto free_skb to goto drop, so that drop statistics will be
updated. Redirect early failures to drop_stats_only, which doesn't
need to worry about skb as it wouldn't be allocated yet.

Cc: Chuang Wang <nashuiliang@xxxxxxxxx>
Signed-off-by: Jon Kohler <jon@xxxxxxxxxxx>
---
 drivers/net/tun.c | 53 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 40 insertions(+), 13 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index e0f5e1fe4bd0..97f130bc5fed 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1657,6 +1657,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
 		}
 		err = tun_xdp_act(tun, xdp_prog, &xdp, act);
 		if (err < 0) {
+			/* tun_xdp_act already handles drop statistics */
 			if (act == XDP_REDIRECT || act == XDP_TX)
 				put_page(alloc_frag->page);
 			goto out;
@@ -1720,12 +1721,17 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 	gso = (struct virtio_net_hdr *)&hdr;
 
 	if (!(tun->flags & IFF_NO_PI)) {
-		if (len < sizeof(pi))
-			return -EINVAL;
+		if (len < sizeof(pi)) {
+			err = -EINVAL;
+			goto drop_stats_only;
+		}
+
 		len -= sizeof(pi);
 
-		if (!copy_from_iter_full(&pi, sizeof(pi), from))
-			return -EFAULT;
+		if (!copy_from_iter_full(&pi, sizeof(pi), from)) {
+			err = -EFAULT;
+			goto drop_stats_only;
+		}
 	}
 
 	if (tun->flags & IFF_VNET_HDR) {
@@ -1734,16 +1740,20 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 		features = tun_vnet_hdr_guest_features(vnet_hdr_sz);
 		hdr_len = __tun_vnet_hdr_get(vnet_hdr_sz, tun->flags,
 					     features, from, gso);
-		if (hdr_len < 0)
-			return hdr_len;
+		if (hdr_len < 0) {
+			err = hdr_len;
+			goto drop_stats_only;
+		}
 
 		len -= vnet_hdr_sz;
 	}
 
 	if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP) {
 		align += NET_IP_ALIGN;
-		if (unlikely(len < ETH_HLEN || (hdr_len && hdr_len < ETH_HLEN)))
-			return -EINVAL;
+		if (unlikely(len < ETH_HLEN || (hdr_len && hdr_len < ETH_HLEN))) {
+			err = -EINVAL;
+			goto drop_stats_only;
+		}
 	}
 
 	good_linear = SKB_MAX_HEAD(align);
@@ -1769,9 +1779,18 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 		 */
 		skb = tun_build_skb(tun, tfile, from, gso, len, &skb_xdp);
 		err = PTR_ERR_OR_ZERO(skb);
-		if (err)
+		if (err) {
+			drop_reason = err == -ENOMEM ?
+				SKB_DROP_REASON_NOMEM :
+				SKB_DROP_REASON_SKB_UCOPY_FAULT;
 			goto drop;
+		}
 		if (!skb)
+			/* tun_build_skb can return null with no err ptr
+			 * from XDP paths, return total_len and always
+			 * appear successful to caller, as drop statistics
+			 * are already handled.
+			 */
 			return total_len;
 	} else {
 		if (!zerocopy) {
@@ -1796,8 +1815,10 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 		}
 
 		err = PTR_ERR_OR_ZERO(skb);
-		if (err)
+		if (err) {
+			drop_reason = SKB_DROP_REASON_NOMEM;
 			goto drop;
+		}
 
 		if (zerocopy)
 			err = zerocopy_sg_from_iter(skb, from);
@@ -1814,7 +1835,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
 		atomic_long_inc(&tun->rx_frame_errors);
 		err = -EINVAL;
-		goto free_skb;
+		drop_reason = SKB_DROP_REASON_DEV_HDR;
+		goto drop;
 	}
 
 	switch (tun->flags & TUN_TYPE_MASK) {
@@ -1831,6 +1853,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 				break;
 			default:
 				err = -EINVAL;
+				drop_reason = SKB_DROP_REASON_INVALID_PROTO;
 				goto drop;
 			}
 		}
@@ -1938,7 +1961,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 			spin_unlock_bh(&queue->lock);
 			rcu_read_unlock();
 			err = -EBUSY;
-			goto free_skb;
+			drop_reason = SKB_DROP_REASON_DEV_READY;
+			goto drop;
 		}
 
 		__skb_queue_tail(queue, skb);
@@ -1969,7 +1993,6 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 	if (err != -EAGAIN)
 		dev_core_stats_rx_dropped_inc(tun->dev);
 
-free_skb:
 	if (!IS_ERR_OR_NULL(skb))
 		kfree_skb_reason(skb, drop_reason);
 
@@ -1980,6 +2003,10 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 	}
 
 	return err ?: total_len;
+
+drop_stats_only:
+	dev_core_stats_rx_dropped_inc(tun->dev);
+	return err;
 }
 
 static ssize_t tun_chr_write_iter(struct kiocb *iocb, struct iov_iter *from)
-- 
2.43.0