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

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] [day] [month] [year] [list]

Message-ID: <1496519338.3992.38.camel@decadent.org.uk>
Date:   Sat, 03 Jun 2017 20:48:58 +0100
From:   Ben Hutchings <ben@...adent.org.uk>
To:     Sven Eckelmann <sven@...fation.org>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        akpm@...ux-foundation.org, Simon Wunderlich <sw@...onwunderlich.de>
Subject: Re: [PATCH 3.16 144/212] batman-adv: Fix double free during
 fragment merge error

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


Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)