Attachment 221983 Details for Bug 232192 – proposed patch

[patch] proposed patch

udp6_input.diff (text/plain), 2.86 KB, created by Andrey V. Elsukov on 2021-01-28 09:15:36 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrey V. Elsukov

Created: 2021-01-28 09:15:36 UTC

Size: 2.86 KB

patch

obsolete

>commit 5d96a8c86309513e60f47a390133f5babc4fa19a
>Author: Andrey V. Elsukov <bu7cher@yandex.ru>
>Date:   Thu Jan 28 12:12:10 2021 +0300
>
>    Fix possible panic in udp6_input() due to lack of locking.
>    
>    The lookup for a IPv6 multicast addresses corresponding to
>    the destination addresses in the datagram is protected by
>    the NET_EPOCH section. Access to each PCB is protected by
>    INP_RLOCK during comparing. But access to socket's so_options
>    field is not protected. And in some cases it is possible,
>    that PCB pointer is still valid, but inp_socket is not.
>    The patch wides lock holding to protect access to inp_socket.
>    Also remove the redundant IN6_IS_ADDR_MULTICAST() check.
>
>diff --git a/sys/netinet6/udp6_usrreq.c b/sys/netinet6/udp6_usrreq.c
>index 1535be90e1b..b32aedf1f02 100644
>--- a/sys/netinet6/udp6_usrreq.c
>+++ b/sys/netinet6/udp6_usrreq.c
>@@ -315,6 +315,7 @@ udp6_input(struct mbuf **mp, int *offp, int proto)
> 		struct inpcb *last;
> 		struct inpcbhead *pcblist;
> 		struct ip6_moptions *imo;
>+		bool inp_locked = false;
> 
> 		/*
> 		 * In the event that laddr should be set to the link-local
>@@ -365,7 +366,7 @@ udp6_input(struct mbuf **mp, int *offp, int proto)
> 			 * and source-specific multicast. [RFC3678]
> 			 */
> 			imo = inp->in6p_moptions;
>-			if (imo && IN6_IS_ADDR_MULTICAST(&ip6->ip6_dst)) {
>+			if (imo != NULL) {
> 				struct sockaddr_in6	 mcaddr;
> 				int			 blocked;
> 
>@@ -392,8 +393,7 @@ udp6_input(struct mbuf **mp, int *offp, int proto)
> 					INP_RUNLOCK(inp); /* XXX */
> 					continue;
> 				}
>-
>-				INP_RUNLOCK(inp);
>+				inp_locked = true;
> 			}
> 			if (last != NULL) {
> 				struct mbuf *n;
>@@ -409,6 +409,8 @@ udp6_input(struct mbuf **mp, int *offp, int proto)
> 							UDP_PROBE(receive, NULL, last,
> 							    ip6, last, uh);
> 						if (udp6_append(last, n, off, fromsa)) {
>+							if (inp_locked)
>+								INP_RUNLOCK(inp);
> 							/* XXX-BZ do we leak m here? */
> 							*mp = NULL;
> 							return (IPPROTO_DONE);
>@@ -426,9 +428,22 @@ udp6_input(struct mbuf **mp, int *offp, int proto)
> 			 * non-shared port.  It assumes that an application
> 			 * will never clear these options after setting them.
> 			 */
>+			if (!inp_locked) {
>+				INP_RLOCK(last);
>+				if (__predict_true(last->inp_flags2 & INP_FREED) != 0) {
>+					INP_RUNLOCK(last);
>+					continue;
>+				}
>+				inp_locked = true;
>+			}
> 			if ((last->inp_socket->so_options &
> 			     (SO_REUSEPORT|SO_REUSEPORT_LB|SO_REUSEADDR)) == 0)
> 				break;
>+
>+			if (inp_locked) {
>+				INP_RUNLOCK(last);
>+				inp_locked = false;
>+			}
> 		}
> 
> 		if (last == NULL) {
>@@ -441,7 +456,10 @@ udp6_input(struct mbuf **mp, int *offp, int proto)
> 			UDPSTAT_INC(udps_noportmcast);
> 			goto badunlocked;
> 		}
>-		INP_RLOCK(last);
>+
>+		if (!inp_locked)
>+			INP_RLOCK(last);
>+
> 		if (__predict_true(last->inp_flags2 & INP_FREED) == 0) {
> 			if (nxt == IPPROTO_UDPLITE)
> 				UDPLITE_PROBE(receive, NULL, last, ip6, last, uh);

Actions: View | Diff

Attachments on bug 232192: 221454 | 221493 | 221983