Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] xhci: fix memleak on setup address fails.
To:     Ikjoon Jang <ikjn@chromium.org>
Cc:     Mathias Nyman <mathias.nyman@intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20190811082259.48176-1-ikjn@chromium.org>
 <5883d03d-31c4-206a-26c1-ca641dbf845c@linux.intel.com>
 <CAATdQgA4z+hqM=U+1dRLQOq7obH3kq7C+pR8BzmzfvskxNKRng@mail.gmail.com>
From:   Mathias Nyman <mathias.nyman@linux.intel.com>
Message-ID: <ddcb3b03-90c6-876e-31ee-005be6eebd1a@linux.intel.com>
Date:   Fri, 30 Aug 2019 16:40:25 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <CAATdQgA4z+hqM=U+1dRLQOq7obH3kq7C+pR8BzmzfvskxNKRng@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 26.8.2019 9.41, Ikjoon Jang wrote:
> On Wed, Aug 14, 2019 at 9:57 PM Mathias Nyman
> <mathias.nyman@linux.intel.com> wrote:
>>
>> On 11.8.2019 11.22, Ikjoon Jang wrote:
>>> Xhci re-enables a slot on transaction error in set_address using
>>> xhci_disable_slot() + xhci_alloc_dev().
>>>
>>> But in this case, xhci_alloc_dev() creates debugfs entries upon an
>>> existing device without cleaning up old entries, thus memory leaks.
>>>
>>> So this patch simply moves calling xhci_debugfs_free_dev() from
>>> xhci_free_dev() to xhci_disable_slot().
>>>
>>
>> Othwerwise this looks good, but xhci_alloc_dev() will call xhci_disable_slot()
>> in some failure cases before the slot debugfs entry is created.
>>
>> In these cases xhci_debugfs_remove_slot() will be called without
>> xhci_debugfs_create_slot() ever being called.
>>
>> This might not be an issue as xhci_debugfs_remove_slot() checks
>> if (!dev || !dev->debugfs_private) before doing anything, but should
>> be checked out.
>>
> 
> I checked out the case by adding simple fault injection on xhci_alloc_dev(),
> to simulate xhci_debugfs_remove_slot() can be called without
> xhci_debugfs_create_slot() being called.
> 

Thanks, patch sent forward

-Mathias