diff mbox series

[v3,net-next,3/7] ipv6: Consolidate option cases in ip6_datagram_send_ctl

Message ID 1555350740-23490-4-git-send-email-tom@quantonium.net
State Changes Requested
Delegated to: David Miller
Headers show
Series exthdrs: Make ext. headers & options useful - Part I | expand

Commit Message

Tom Herbert April 15, 2019, 5:52 p.m. UTC 
Consolidate cases for IPV6_2292HOPOPTS, IPV6_HOPOPTS, IPV6_2292DSTOPTS,
IPV6_DSTOPTS, and IPV6_RTHDRDSTOPTS. Most of the work and verifications
are common for all these case, individual differences in processing can
be implemented with an embedded switch statement.
---
 net/ipv6/datagram.c | 66 ++++++++++++++++++-----------------------------------
 1 file changed, 22 insertions(+), 44 deletions(-)

Comments

David Miller April 17, 2019, 4 a.m. UTC | #1 
From: Tom Herbert <tom@herbertland.com>
Date: Mon, 15 Apr 2019 10:52:16 -0700

>  		case IPV6_2292HOPOPTS:
>  		case IPV6_HOPOPTS:
> -			if (opt->hopopt || cmsg->cmsg_len < CMSG_LEN(sizeof(struct ipv6_opt_hdr))) {
> -				err = -EINVAL;
> -				goto exit_f;
> -			}

You're consolidated version of this code checks opt->hopopt after
things like ns_capable() thus changing the priorities of the error
conditions.  You code get -EPERM when previously -EINVAL would have
been seen.

I really don't like changes like this, it's so irritating double checking
all of the possible user visible side effects in all of these details.
diff mbox series

Patch

diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index ee4a4e5..f4742db 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -842,49 +842,7 @@  int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
 
 		case IPV6_2292HOPOPTS:
 		case IPV6_HOPOPTS:
-			if (opt->hopopt || cmsg->cmsg_len < CMSG_LEN(sizeof(struct ipv6_opt_hdr))) {
-				err = -EINVAL;
-				goto exit_f;
-			}
-
-			hdr = (struct ipv6_opt_hdr *)CMSG_DATA(cmsg);
-			len = ((hdr->hdrlen + 1) << 3);
-			if (cmsg->cmsg_len < CMSG_LEN(len)) {
-				err = -EINVAL;
-				goto exit_f;
-			}
-			if (!ns_capable(net->user_ns, CAP_NET_RAW)) {
-				err = -EPERM;
-				goto exit_f;
-			}
-			opt->opt_nflen += len;
-			opt->hopopt = hdr;
-			break;
-
 		case IPV6_2292DSTOPTS:
-			if (cmsg->cmsg_len < CMSG_LEN(sizeof(struct ipv6_opt_hdr))) {
-				err = -EINVAL;
-				goto exit_f;
-			}
-
-			hdr = (struct ipv6_opt_hdr *)CMSG_DATA(cmsg);
-			len = ((hdr->hdrlen + 1) << 3);
-			if (cmsg->cmsg_len < CMSG_LEN(len)) {
-				err = -EINVAL;
-				goto exit_f;
-			}
-			if (!ns_capable(net->user_ns, CAP_NET_RAW)) {
-				err = -EPERM;
-				goto exit_f;
-			}
-			if (opt->dst1opt) {
-				err = -EINVAL;
-				goto exit_f;
-			}
-			opt->opt_flen += len;
-			opt->dst1opt = hdr;
-			break;
-
 		case IPV6_DSTOPTS:
 		case IPV6_RTHDRDSTOPTS:
 			if (cmsg->cmsg_len < CMSG_LEN(sizeof(struct ipv6_opt_hdr))) {
@@ -902,13 +860,33 @@  int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
 				err = -EPERM;
 				goto exit_f;
 			}
-			if (cmsg->cmsg_type == IPV6_DSTOPTS) {
+
+			switch (cmsg->cmsg_type) {
+			case IPV6_2292HOPOPTS:
+			case IPV6_HOPOPTS:
+				if (opt->hopopt) {
+					err = -EINVAL;
+					goto exit_f;
+				}
+				opt->opt_nflen += len;
+				opt->hopopt = hdr;
+				break;
+			case IPV6_2292DSTOPTS:
+				if (opt->dst1opt) {
+					err = -EINVAL;
+					goto exit_f;
+				}
+				/* Fallthrough */
+			case IPV6_DSTOPTS:
 				opt->opt_flen += len;
 				opt->dst1opt = hdr;
-			} else {
+				break;
+			case IPV6_RTHDRDSTOPTS:
 				opt->opt_nflen += len;
 				opt->dst0opt = hdr;
+				break;
 			}
+
 			break;
 
 		case IPV6_2292RTHDR: