Messages in this thread Patch in this message | | | Date | Fri, 24 Oct 2003 18:29:59 +0200 | From | Tomas Szepe <> | Subject | possible bug in tcp_input.c |
| |
Hi David,
We came up with the attached patch during a hectic oops tracing session that got started by our sysadmin writing down an oops using the pen & paper method, no less. The crashing machine has been a firewall running a very unusual NAT + QoS configuration, however we believe that we might have discovered a real bug in 2.4's tcp_input.c. Since our insight into the internals of the tcp/ip stack is far from even basic, we are seeking your opinion on whether we are correct.
The static inline function skb_peek() as defined in include/linux/skbuff.h returns a pointer to a sk_buff, or NULL when its argument is an empty list or a pointer to the head element. Since this is documented behavior, it is not surprising that all segments of code within tcp_input.c dealing with a return from skb_peek() take care not to dereference the returned pointer if it happens to be NULL. There is an exception, though:
/* tcp_input.c, line 1138 */ static inline int tcp_head_timedout(struct sock *sk, struct tcp_opt *tp) { return tp->packets_out && tcp_skb_timedout(tp, skb_peek(&sk->write_queue)); }
The passed NULL (and yes, this is where we are getting one) is dereferenced immediately in:
/* tcp_input.c, line 1133 */ static inline int tcp_skb_timedout(struct tcp_opt *tp, struct sk_buff *skb) { return (tcp_time_stamp - TCP_SKB_CB(skb)->when > tp->rto); }
with TCP_SKB_CB that is defined as
/* tcp.h, line 1034 */ #define TCP_SKB_CB(__skb) ((struct tcp_skb_cb *)&((__skb)->cb[0]))
We are proposing to cure the problem by adding a simple check in tcp_head_timedout(), but are not sure whether this is the right thing to do, because as a friend put it, we seem to be fixing a leaking faucet in a god damn power plant.
Thanks for any help, -- Tomas Szepe <szepe@pinerecords.com>
diff -urN a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c --- a/net/ipv4/tcp_input.c 2003-06-13 16:51:39 +0200 +++ b/net/ipv4/tcp_input.c 2003-10-24 17:41:19 +0200 @@ -1138,7 +1138,11 @@ static inline int tcp_head_timedout(struct sock *sk, struct tcp_opt *tp) { - return tp->packets_out && tcp_skb_timedout(tp, skb_peek(&sk->write_queue)); + struct sk_buff *skb = skb_peek(&sk->write_queue); + if (skb == NULL) + return 1; + + return tp->packets_out && tcp_skb_timedout(tp, skb); } /* Linux NewReno/SACK/FACK/ECN state machine. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |