tcp repair: Fix unaligned access when repairing options (v2)

Don't pick __u8/__u16 values directly from raw pointers, but instead use an array of structures of code:value pairs. This is OK, since the buffer we take options from is not an skb memory, but a user-to-kernel one. For those options which don't require any value now, require this to be zero (for potential future extension of this API). v2: Changed tcp_repair_opt to use two __u32-s as spotted by David Laight. Signed-off-by: Pavel Emelyanov <[email protected]> Signed-off-by: David S. Miller <[email protected]>
goeke · Apr 26, 2012 · de248a7 · de248a7
1 parent 2d31950
commit de248a7
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 39 deletions.
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
@@ -111,6 +111,11 @@ enum {
 #define TCP_QUEUE_SEQ		21
 #define TCP_REPAIR_OPTIONS	22
 
+struct tcp_repair_opt {
+	__u32	opt_code;
+	__u32	opt_val;
+};
+
 enum {
 	TCP_NO_QUEUE,
 	TCP_RECV_QUEUE,

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
@@ -2283,60 +2283,40 @@ static inline int tcp_can_repair_sock(struct sock *sk)
 		((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_ESTABLISHED));
 }
 
-static int tcp_repair_options_est(struct tcp_sock *tp, char __user *optbuf, unsigned int len)
+static int tcp_repair_options_est(struct tcp_sock *tp,
+		struct tcp_repair_opt __user *optbuf, unsigned int len)
 {
-	/*
-	 * Options are stored in CODE:VALUE form where CODE is 8bit and VALUE
-	 * fits the respective TCPOLEN_ size
-	 */
+	struct tcp_repair_opt opt;
 
-	while (len > 0) {
-		u8 opcode;
-
-		if (get_user(opcode, optbuf))
+	while (len >= sizeof(opt)) {
+		if (copy_from_user(&opt, optbuf, sizeof(opt)))
 			return -EFAULT;
 
 		optbuf++;
-		len--;
-
-		switch (opcode) {
-		case TCPOPT_MSS: {
-			u16 in_mss;
+		len -= sizeof(opt);
 
-			if (len < sizeof(in_mss))
-				return -ENODATA;
-			if (get_user(in_mss, optbuf))
-				return -EFAULT;
-
-			tp->rx_opt.mss_clamp = in_mss;
-
-			optbuf += sizeof(in_mss);
-			len -= sizeof(in_mss);
+		switch (opt.opt_code) {
+		case TCPOPT_MSS:
+			tp->rx_opt.mss_clamp = opt.opt_val;
 			break;
-		}
-		case TCPOPT_WINDOW: {
-			u8 wscale;
-
-			if (len < sizeof(wscale))
-				return -ENODATA;
-			if (get_user(wscale, optbuf))
-				return -EFAULT;
-
-			if (wscale > 14)
+		case TCPOPT_WINDOW:
+			if (opt.opt_val > 14)
 				return -EFBIG;
 
-			tp->rx_opt.snd_wscale = wscale;
-
-			optbuf += sizeof(wscale);
-			len -= sizeof(wscale);
+			tp->rx_opt.snd_wscale = opt.opt_val;
 			break;
-		}
 		case TCPOPT_SACK_PERM:
+			if (opt.opt_val != 0)
+				return -EINVAL;
+
 			tp->rx_opt.sack_ok |= TCP_SACK_SEEN;
 			if (sysctl_tcp_fack)
 				tcp_enable_fack(tp);
 			break;
 		case TCPOPT_TIMESTAMP:
+			if (opt.opt_val != 0)
+				return -EINVAL;
+
 			tp->rx_opt.tstamp_ok = 1;
 			break;
 		}
@@ -2557,7 +2537,9 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
 		if (!tp->repair)
 			err = -EINVAL;
 		else if (sk->sk_state == TCP_ESTABLISHED)
-			err = tcp_repair_options_est(tp, optval, optlen);
+			err = tcp_repair_options_est(tp,
+					(struct tcp_repair_opt __user *)optval,
+					optlen);
 		else
 			err = -EPERM;
 		break;