| Message ID | 20120919144557.16956.11280.stgit@localhost.localdomain |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Alan Cox <alan@lxorguk.ukuu.org.uk> Date: Wed, 19 Sep 2012 15:46:06 +0100 > From: Alan Cox <alan@linux.intel.com> > > Both tcp_timewait_state_process and tcp_check_req use the same basic > construct of > > struct tcp_options received tmp_opt; > tmp_opt.saw_tstamp = 0; > > then call > > tcp_parse_options > > However if they are fed a frame containing a TCP_SACK then tbe code > behaviour is undefined because opt_rx->sack_ok is undefined data. > > This ought to be documented if it is intentional. > > Signed-off-by: Alan Cox <alan@linux.intel.com> Applied to net-next, except I took this hunk out: > @@ -96,6 +98,7 @@ tcp_timewait_state_process(struct inet_timewait_sock *tw, struct sk_buff *skb, > bool paws_reject = false; > > tmp_opt.saw_tstamp = 0; > + > if (th->doff > (sizeof(*th) >> 2) && tcptw->tw_ts_recent_stamp) { > tcp_parse_options(skb, &tmp_opt, &hash_location, 0, NULL); > Since it's unrelated to your change, and if you were going to do this in tcp_timewait_state_process() you should do it in tcp_check_req() as well since the code is identical. Longer term maybe we probably should add a tcp_minisock_parse_options() that elides TCP_SACK and other bits these cases do not want. Thanks Alan. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c index e965319..a4ace80 100644 --- a/net/ipv4/tcp_minisocks.c +++ b/net/ipv4/tcp_minisocks.c @@ -85,6 +85,8 @@ static bool tcp_in_window(u32 seq, u32 end_seq, u32 s_win, u32 e_win) * spinlock it. I do not want! Well, probability of misbehaviour * is ridiculously low and, seems, we could use some mb() tricks * to avoid misread sequence numbers, states etc. --ANK + * + * We don't need to initialize tmp_out.sack_ok as we don't use the results */ enum tcp_tw_status tcp_timewait_state_process(struct inet_timewait_sock *tw, struct sk_buff *skb, @@ -96,6 +98,7 @@ tcp_timewait_state_process(struct inet_timewait_sock *tw, struct sk_buff *skb, bool paws_reject = false; tmp_opt.saw_tstamp = 0; + if (th->doff > (sizeof(*th) >> 2) && tcptw->tw_ts_recent_stamp) { tcp_parse_options(skb, &tmp_opt, &hash_location, 0, NULL); @@ -522,6 +525,8 @@ EXPORT_SYMBOL(tcp_create_openreq_child); * * XXX (TFO) - The current impl contains a special check for ack * validation and inside tcp_v4_reqsk_send_ack(). Can we do better? + * + * We don't need to initialize tmp_opt.sack_ok as we don't use the results */ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,