netdev - Re: [PATCH 12/21] [TCP]: Introduce per skb fack

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.64.0712021321410.6477@kivilampi-30.cs.helsinki.fi>
Date:	Sun, 2 Dec 2007 13:29:36 +0200 (EET)
From:	"Ilpo Järvinen" <ilpo.jarvinen@...sinki.fi>
To:	Herbert Xu <herbert@...dor.apana.org.au>
cc:	David Miller <davem@...emloft.net>,
	Netdev <netdev@...r.kernel.org>,
	Stephen Hemminger <shemminger@...ux-foundation.org>
Subject: Re: [PATCH 12/21] [TCP]: Introduce per skb fack_counts to retransmit
 queue

On Sun, 2 Dec 2007, Herbert Xu wrote:

> On Sun, Dec 02, 2007 at 12:48:07AM +0200, Ilpo Järvinen wrote:
> >
> > @@ -1220,6 +1221,11 @@ static inline struct sk_buff *tcp_write_queue_next(struct sock *sk, struct sk_bu
> >  	return skb->next;
> >  }
> > 
> > +static inline struct sk_buff *tcp_write_queue_prev(struct sock *sk, struct sk_buff *skb)
> > +{
> > +	return skb->prev;
> > +}
> > +
> >  #define tcp_for_write_queue(skb, sk)					\
> >  		for (skb = (sk)->sk_write_queue.next;			\
> >  		     (skb != (struct sk_buff *)&(sk)->sk_write_queue);	\
> > @@ -1241,6 +1247,11 @@ static inline struct sk_buff *tcp_send_head(struct sock *sk)
> > 
> >  static inline void tcp_advance_send_head(struct sock *sk, struct sk_buff *skb)
> >  {
> > +	struct sk_buff *prev = tcp_write_queue_prev(sk, skb);
> > +
> > +	TCP_SKB_CB(skb)->fack_count = TCP_SKB_CB(prev)->fack_count +
> > +				      tcp_skb_pcount(prev);
> > +
> >  	sk->sk_send_head = skb->next;
> >  	if (sk->sk_send_head == (struct sk_buff *)&sk->sk_write_queue)
> >  		sk->sk_send_head = NULL;
> 
> This crashed my machine on boot.  The reason is that prev was empty.
> I've added this fix to net-2.6.25.

Hmm didn't crash for me, probably a silent corruption instead :-(.

> [TCP]: Fix crash in tcp_advance_send_head
> 
> We need to check whether there is a prev in tcp_advance_send_head
> before dereferencing it. This patch does just that and leaves the
> fack count at zero if there is no prev.

Maybe I could add catch for invalid skb dereferences (those list heads) to 
TCP_SKB_CB and tcp_skb_pcount if some DEBUG thing is set. Those two should 
catch at least 98% of the invalid users.

...Not sure if CONFIG_DEBUG_LIST is right one to use, suggestion welcome.

-- 
 i.