netdev - Re: [IPv6] BUG: NULL pointer dereference in(?) ip6_flush_pending

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]

Date:	Mon, 10 Sep 2007 08:38:47 +0900 (JST)
From:	YOSHIFUJI Hideaki / 吉藤英明 
	<yoshfuji@...ux-ipv6.org>
To:	berni@...kenwald.de
Cc:	netdev@...r.kernel.org, yoshfuji@...ux-ipv6.org,
	miyazawa@...ux-ipv6.org
Subject: Re: [IPv6] BUG: NULL pointer dereference in(?)
 ip6_flush_pending_frames

In article <20070909222400.GC7591@...lix.birkenwald.de> (at Mon, 10 Sep 2007 00:24:00 +0200), Bernhard Schmidt <berni@...kenwald.de> says:

> I'm running a public Teredo relay (IPv4-to-IPv6 migration protocol)
> using Miredo. Every once in a while (a few minutes to days after
> daemon restart) it becomes unusable and I see the following kernel
> message:
> 
> BUG: unable to handle kernel NULL pointer dereference at virtual address
> 0000008c
:
> EIP is at ip6_flush_pending_frames+0x97/0x121

I think I've found a bug.

Some of skbs in sk->write_queue do not have skb->dst because
we do not fill skb->dst when we allocate new skb in append_data().

Miyazawa-san, am I right?

BTW, I think we may not need to (or we should not) increment some stats
when using corking; if 100 sendmsg() (with MSG_MORE) result in 2 packets,
how many should we increment?

If 100, we should set skb->dst for every queued skbs.

If 1 (or 2 (*)), we increment the stats for the first queued skb and
we should just skip incrementing OutDiscards for the rest of queued skbs,
adn we should also impelement this semantics in other places;
e.g., we should increment other stats just once, not 100 times.

*: depends on the place we are discarding the datagram.

I guess should just increment by 1 (or 2).

Anyway, please try this.

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@...ux-ipv6.org>

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 4704b5f..e489499 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1423,8 +1423,9 @@ void ip6_flush_pending_frames(struct sock *sk)
 	struct sk_buff *skb;

 	while ((skb = __skb_dequeue_tail(&sk->sk_write_queue)) != NULL) {
-		IP6_INC_STATS(ip6_dst_idev(skb->dst),
-			      IPSTATS_MIB_OUTDISCARDS);
+		if (skb->dst)
+			IP6_INC_STATS(ip6_dst_idev(skb->dst),
+				      IPSTATS_MIB_OUTDISCARDS);
 		kfree_skb(skb);
 	}

--yoshfuji
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html