On Sun, Jan 14, 2018 at 11:07 PM, Jason Wang <jasow...@redhat.com> wrote: > > > On 2018年01月14日 01:31, Cong Wang wrote: >> >> On Thu, Jan 11, 2018 at 2:16 AM, Jason Wang <jasow...@redhat.com> wrote: >>> >>> It looks to me what is actual missed is the cleanups tun_detach_all(). >>> For >>> me the only case that could leak is >>> >>> open >>> attach >>> ip link del link dev tap0 >>> close or another set_iff() >>> >>> So in this case, clean during close is not sufficient since it could be >>> attached to another device. >> >> In this case, close() still calls tun_detach() with clean=true, so >> with my patch, the tx_array is still cleaned. What am I missing here? >> Are you implying clean=true is not sufficient? > > > Consider the corner case: > > 1) open > 2) tun_set_iff() (which calls tun_attach to initialize skb_array) > 3) ip link del link dev tap0 (which calls tun_detach_all()) > 4) tun_set_iff() (current codes does not forbid this and it will allocate > skb array again) > > Consider the skb array was only initialized when attach it to a real device, > we should do the cleanup when we detach it from a device which happens on > two places: > > - actively: close to an tun fd (__tun_deatch()) > - passively: tun device was destroyed (tun_detach_all())
Fair enough, I will send out v3. Thanks.