Re: [PATCH 3.16 144/212] batman-adv: Fix double free during fragment merge error

[Ben Hutchings]

On Thu, 2017-06-01 at 18:44 +0200, Sven Eckelmann wrote:
> On Donnerstag, 1. Juni 2017 16:43:16 CEST Ben Hutchings wrote:
> > 3.16.44-rc1 review patch.ÂÂIf anyone has any objections, please let me know.
> 
> It looks to me like there are problems with this backport. The surroundingÂ
> code has to be adjusted slightly further to avoid additional problems.

Thanks for the review.

[...]
> It is not really easy to see but this change will introduce a new double freeÂ
> for kernels older than v4.10. The relevant commit is b91a2543b4c1 ("batman-
> adv: Consume skb in receive handlers"). This was discussed in following gluonÂ
> ticket https://github.com/freifunk-gluon/gluon/issues/1083 (just in case youÂ
> are interested about the details)
> 
> Following change must therefore be added to this patch on older kernels:
> 
> ÂÂÂÂ--- a/net/batman-adv/routing.c
> ÂÂÂÂ+++ b/net/batman-adv/routing.c
> ÂÂÂÂ@@ -961,6 +961,12 @@ int batadv_recv_frag_packet(struct sk_buff *skb,
> ÂÂÂÂÂ	batadv_inc_counter(bat_priv, BATADV_CNT_FRAG_RX);
> ÂÂÂÂÂ	batadv_add_counter(bat_priv, BATADV_CNT_FRAG_RX_BYTES, skb->len);
> ÂÂÂÂÂ
> ÂÂÂÂ+	/* batadv_frag_skb_buffer will always consume the skb and
> ÂÂÂÂ+	Â* the caller should therefore never try to free the
> ÂÂÂÂ+	Â* skb after this point
> ÂÂÂÂ+	Â*/
> ÂÂÂÂ+	ret = NET_RX_SUCCESS;
> ÂÂÂÂ+
> ÂÂÂÂÂ	/* Add fragment to buffer and merge if possible. */
> ÂÂÂÂÂ	if (!batadv_frag_skb_buffer(&skb, orig_node_src))
> ÂÂÂÂÂ		goto out;
> 
> You can also remove the same instruction which appears later in this function.

OK, I'll squash this into the patch.

Ben.

-- 
Ben Hutchings
Experience is directly proportional to the value of equipment
destroyed.
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ- Carolyn
Scheppner

Attachment: signature.asc
Description: This is a digitally signed message part