On Fri, Jun 2, 2017 at 2:25 PM, Craig Gallek <kraigatg...@gmail.com> wrote: > On Fri, Jun 2, 2017 at 2:05 PM, David Miller <da...@davemloft.net> wrote: >> From: Ben Hutchings <b...@decadent.org.uk> >> Date: Wed, 31 May 2017 13:26:02 +0100 >> >>> If I'm not mistaken, ipv6_gso_segment() now leaks segs if >>> ip6_find_1stfragopt() fails. I'm not sure whether the fix would be as >>> simple as adding a kfree_skb(segs) or whether more complex cleanup is >>> needed. >> >> I think we need to use kfree_skb_list(), like the following. > I think this is problematic as well. ipv6_gso_segment could > previously return errors, in which case the caller uses kfree_skb (ex > validate_xmit_skb() -> skb_gso_segment -> ... > callbacks.gso_segment()). Having the kfree_skb_list here would cause > a double free if I'm reading this correctly. > > My first guess was going to be skb_gso_error_unwind(), but I'm still > trying to understand that code... > > Sorry again for the fallout from this bug fix. This is my first time > down this code path and I clearly didn't understand it fully :/
Ok, I take it back. I believe your kfree_skb_list suggestion is correct. I was assuming that skb_segment consumed the original skb upon successful segmentation. It does not. This is exactly why validate_xmit_skb calls consume_skb when segments are returned. Further, there is at least one existing example of kfree_skb_list in a similar post-semgent cleanup path (esp6_gso_segment).
