From: Craig Gallek <kraigatg...@gmail.com> Date: Fri, 2 Jun 2017 15:27:41 -0400
> 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). Great, thanks for reviewing. I've pushed this into 'net' and queued it up for -stable.
