Skip to contentSkip to conversation

OOPS with NETLINK_FIB_LOOKUP netlink socket

PATCH In review Denis V. Lunev 3 participants 4 messages Dec 21, 2007

Subsystems (3)
Networking Network drivers IPv4/IPv6
lore
Share & tools
lore

Denis V. Lunev

author
#0 lore raw

PATCH 1/1 OOPS with NETLINK_FIB_LOOKUP netlink socket

nl_fib_input re-reuses incoming skb to send the reply. This means that this packet will be freed twice, namely in:
- netlink_unicast_kernel
- on receive path
Use clone to send as a cure, the caller is responsible for kfree_skb on error.

Thanks to Alexey Dobryan, who originally found the problem.

Signed-off-by Denis V. Lunev <[email protected]>

---

net/ipv4/fib_frontend.c +6 -3
index b03c4c6..ac6238a 100644
@@ -832,10 +832,13 @@ static void nl_fib_input(struct sk_buff *skb)
 
 	nlh = nlmsg_hdr(skb);
 	if (skb->len < NLMSG_SPACE(0) || skb->len < nlh->nlmsg_len ||
-	    nlh->nlmsg_len < NLMSG_LENGTH(sizeof(*frn))) {
-		kfree_skb(skb);
+	    nlh->nlmsg_len < NLMSG_LENGTH(sizeof(*frn)))
 		return;
-	}
+
+	skb = skb_clone(skb, GFP_KERNEL);
+	if (skb == NULL)
+		return;
+	nlh = nlmsg_hdr(skb);
 
 	frn = (struct fib_result_nl *) NLMSG_DATA(nlh);
 	tb = fib_get_table(frn->tb_id_in);
maintainer
#1 lore raw

From: "Denis V. Lunev" <[email protected]>
Date: Fri, 21 Dec 2007 12:00:43 +0300

Quoted 9 lines

nl_fib_input re-reuses incoming skb to send the reply. This means that this packet will be freed twice, namely in:
- netlink_unicast_kernel
- on receive path
Use clone to send as a cure, the caller is responsible for kfree_skb on error.

Thanks to Alexey Dobryan, who originally found the problem.

Signed-off-by Denis V. Lunev <[email protected]>

What introduced this bug? This code didn't have this problem previously.

#2 lore raw
Quoted 9 lines · David Miller

From: "Denis V. Lunev" <[email protected]>
Date: Fri, 21 Dec 2007 12:00:43 +0300

Quoted 6 lines

nl_fib_input re-reuses incoming skb to send the reply. This means that this packet will be freed twice, namely in:
- netlink_unicast_kernel
- on receive path
Use clone to send as a cure, the caller is responsible for kfree_skb on error.

Quoted 7 lines
Quoted 3 lines

Thanks to Alexey Dobryan, who originally found the problem.

Signed-off-by Denis V. Lunev <[email protected]>

What introduced this bug? This code didn't have this problem previously.

commit cd40b7d3983c708aabe3d3008ec64ffce56d33b0

Author: Denis V. Lunev <[email protected]>
Date:   Wed Oct 10 21:15:29 2007 -0700
maintainer
#3 lore raw

From: "Denis V. Lunev" <[email protected]>
Date: Fri, 21 Dec 2007 12:39:36 +0300

Quoted 7 lines
Quoted 2 lines · David Miller

What introduced this bug? This code didn't have this problem previously.

commit cd40b7d3983c708aabe3d3008ec64ffce56d33b0

Author: Denis V. Lunev <[email protected]>
Date:   Wed Oct 10 21:15:29 2007 -0700

Thank you, I'm going to add this reference to the changeset comments so that it will help others who look at this fix.

Consider your patch applied, thanks! :-)