udp: Fix udp_poll() and ioctl()

udp_poll() can in some circumstances drop frames with incorrect checksums. Problem is we now have to lock the socket while dropping frames, or risk sk_forward corruption. This bug is present since commit 95766fff6b9a78d1 ([UDP]: Add memory accounting.) While we are at it, we can correct ioctl(SIOCINQ) to also drop bad frames. Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>

udp: Fix udp_poll() and ioctl()
udp_poll() can in some circumstances drop frames with incorrect checksums. Problem is we now have to lock the socket while dropping frames, or risk sk_forward corruption. This bug is present since commit 95766fff6b9a78d1 ([UDP]: Add memory accounting.) While we are at it, we can correct ioctl(SIOCINQ) to also drop bad frames. Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Eric Dumazet · David S. Miller
1 parent 9652041da1
Showing 1 changed file with 43 additions and 30 deletions Side-by-side Diff
net/ipv4/udp.c
@@ -841,6 +841,42 @@
 	return ret;
 }
  
+
+/**
+ *	first_packet_length	- return length of first packet in receive queue
+ *	@sk: socket
+ *
+ *	Drops all bad checksum frames, until a valid one is found.
+ *	Returns the length of found skb, or 0 if none is found.
+ */
+static unsigned int first_packet_length(struct sock *sk)
+{
+	struct sk_buff_head list_kill, *rcvq = &sk->sk_receive_queue;
+	struct sk_buff *skb;
+	unsigned int res;
+
+	__skb_queue_head_init(&list_kill);
+
+	spin_lock_bh(&rcvq->lock);
+	while ((skb = skb_peek(rcvq)) != NULL &&
+		udp_lib_checksum_complete(skb)) {
+		UDP_INC_STATS_BH(sock_net(sk), UDP_MIB_INERRORS,
+				 IS_UDPLITE(sk));
+		__skb_unlink(skb, rcvq);
+		__skb_queue_tail(&list_kill, skb);
+	}
+	res = skb ? skb->len : 0;
+	spin_unlock_bh(&rcvq->lock);
+
+	if (!skb_queue_empty(&list_kill)) {
+		lock_sock(sk);
+		__skb_queue_purge(&list_kill);
+		sk_mem_reclaim_partial(sk);
+		release_sock(sk);
+	}
+	return res;
+}
+
 /*
  *	IOCTL requests applicable to the UDP protocol
  */
  
  
@@ -857,21 +893,16 @@
  
 	case SIOCINQ:
 	{
-		struct sk_buff *skb;
-		unsigned long amount;
+		unsigned int amount = first_packet_length(sk);
  
-		amount = 0;
-		spin_lock_bh(&sk->sk_receive_queue.lock);
-		skb = skb_peek(&sk->sk_receive_queue);
-		if (skb != NULL) {
+		if (amount)
 			/*
 			 * We will only return the amount
 			 * of this packet since that is all
 			 * that will be read.
 			 */
-			amount = skb->len - sizeof(struct udphdr);
-		}
-		spin_unlock_bh(&sk->sk_receive_queue.lock);
+			amount -= sizeof(struct udphdr);
+
 		return put_user(amount, (int __user *)arg);
 	}
  
  
@@ -1540,29 +1571,11 @@
 {
 	unsigned int mask = datagram_poll(file, sock, wait);
 	struct sock *sk = sock->sk;
-	int 	is_lite = IS_UDPLITE(sk);
  
 	/* Check for false positives due to checksum errors */
-	if ((mask & POLLRDNORM) &&
-	    !(file->f_flags & O_NONBLOCK) &&
-	    !(sk->sk_shutdown & RCV_SHUTDOWN)) {
-		struct sk_buff_head *rcvq = &sk->sk_receive_queue;
-		struct sk_buff *skb;
-
-		spin_lock_bh(&rcvq->lock);
-		while ((skb = skb_peek(rcvq)) != NULL &&
-		       udp_lib_checksum_complete(skb)) {
-			UDP_INC_STATS_BH(sock_net(sk),
-					UDP_MIB_INERRORS, is_lite);
-			__skb_unlink(skb, rcvq);
-			kfree_skb(skb);
-		}
-		spin_unlock_bh(&rcvq->lock);
-
-		/* nothing to see, move along */
-		if (skb == NULL)
-			mask &= ~(POLLIN | POLLRDNORM);
-	}
+	if ((mask & POLLRDNORM) && !(file->f_flags & O_NONBLOCK) &&
+	    !(sk->sk_shutdown & RCV_SHUTDOWN) && !first_packet_length(sk))
+		mask &= ~(POLLIN | POLLRDNORM);
  
 	return mask;
...	...	@@ -841,6 +841,42 @@
841	841	return ret;
842	842	}
843	843
	844	+
	845	+/**
	846	+ * first_packet_length - return length of first packet in receive queue
	847	+ * @sk: socket
	848	+ *
	849	+ * Drops all bad checksum frames, until a valid one is found.
	850	+ * Returns the length of found skb, or 0 if none is found.
	851	+ */
	852	+static unsigned int first_packet_length(struct sock *sk)
	853	+{
	854	+ struct sk_buff_head list_kill, *rcvq = &sk->sk_receive_queue;
	855	+ struct sk_buff *skb;
	856	+ unsigned int res;
	857	+
	858	+ __skb_queue_head_init(&list_kill);
	859	+
	860	+ spin_lock_bh(&rcvq->lock);
	861	+ while ((skb = skb_peek(rcvq)) != NULL &&
	862	+ udp_lib_checksum_complete(skb)) {
	863	+ UDP_INC_STATS_BH(sock_net(sk), UDP_MIB_INERRORS,
	864	+ IS_UDPLITE(sk));
	865	+ __skb_unlink(skb, rcvq);
	866	+ __skb_queue_tail(&list_kill, skb);
	867	+ }
	868	+ res = skb ? skb->len : 0;
	869	+ spin_unlock_bh(&rcvq->lock);
	870	+
	871	+ if (!skb_queue_empty(&list_kill)) {
	872	+ lock_sock(sk);
	873	+ __skb_queue_purge(&list_kill);
	874	+ sk_mem_reclaim_partial(sk);
	875	+ release_sock(sk);
	876	+ }
	877	+ return res;
	878	+}
	879	+
844	880	/*
845	881	* IOCTL requests applicable to the UDP protocol
846	882	*/
847	883
848	884
...	...	@@ -857,21 +893,16 @@
857	893
858	894	case SIOCINQ:
859	895	{
860		- struct sk_buff *skb;
861		- unsigned long amount;
	896	+ unsigned int amount = first_packet_length(sk);
862	897
863		- amount = 0;
864		- spin_lock_bh(&sk->sk_receive_queue.lock);
865		- skb = skb_peek(&sk->sk_receive_queue);
866		- if (skb != NULL) {
	898	+ if (amount)
867	899	/*
868	900	* We will only return the amount
869	901	* of this packet since that is all
870	902	* that will be read.
871	903	*/
872		- amount = skb->len - sizeof(struct udphdr);
873		- }
874		- spin_unlock_bh(&sk->sk_receive_queue.lock);
	904	+ amount -= sizeof(struct udphdr);
	905	+
875	906	return put_user(amount, (int __user *)arg);
876	907	}
877	908
878	909
...	...	@@ -1540,29 +1571,11 @@
1540	1571	{
1541	1572	unsigned int mask = datagram_poll(file, sock, wait);
1542	1573	struct sock *sk = sock->sk;
1543		- int is_lite = IS_UDPLITE(sk);
1544	1574
1545	1575	/* Check for false positives due to checksum errors */
1546		- if ((mask & POLLRDNORM) &&
1547		- !(file->f_flags & O_NONBLOCK) &&
1548		- !(sk->sk_shutdown & RCV_SHUTDOWN)) {
1549		- struct sk_buff_head *rcvq = &sk->sk_receive_queue;
1550		- struct sk_buff *skb;
1551		-
1552		- spin_lock_bh(&rcvq->lock);
1553		- while ((skb = skb_peek(rcvq)) != NULL &&
1554		- udp_lib_checksum_complete(skb)) {
1555		- UDP_INC_STATS_BH(sock_net(sk),
1556		- UDP_MIB_INERRORS, is_lite);
1557		- __skb_unlink(skb, rcvq);
1558		- kfree_skb(skb);
1559		- }
1560		- spin_unlock_bh(&rcvq->lock);
1561		-
1562		- /* nothing to see, move along */
1563		- if (skb == NULL)
1564		- mask &= ~(POLLIN \| POLLRDNORM);
1565		- }
	1576	+ if ((mask & POLLRDNORM) && !(file->f_flags & O_NONBLOCK) &&
	1577	+ !(sk->sk_shutdown & RCV_SHUTDOWN) && !first_packet_length(sk))
	1578	+ mask &= ~(POLLIN \| POLLRDNORM);
1566	1579
1567	1580	return mask;
1568	1581