From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Wang Subject: Re: [Patch net v2] tun: fix a memory leak for tfile->tx_array Date: Mon, 15 Jan 2018 15:07:41 +0800 Message-ID: <5ac38ff9-af8d-c1c3-fa08-764e2e5491cf@redhat.com> References: <20180110185101.25964-1-xiyou.wangcong@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: Linux Kernel Network Developers , Dmitry Vyukov , "Michael S. Tsirkin" To: Cong Wang Return-path: Received: from mx1.redhat.com ([209.132.183.28]:50874 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751142AbeAOHHt (ORCPT ); Mon, 15 Jan 2018 02:07:49 -0500 In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 2018年01月14日 01:31, Cong Wang wrote: > On Thu, Jan 11, 2018 at 2:16 AM, Jason Wang 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()) Thanks