netdev - Re: [PATCH] net-sysfs: Fix reference count leak

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87tv718rke.fsf@unikie.com>
Date:   Mon, 18 Nov 2019 11:25:05 +0200
From:   jouni.hogander@...kie.com (Jouni Högander)
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     netdev@...r.kernel.org, David Miller <davem@...emloft.net>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>
Subject: Re: [PATCH] net-sysfs: Fix reference count leak

Greg Kroah-Hartman <gregkh@...uxfoundation.org> writes:

> On Fri, Nov 15, 2019 at 02:24:12PM +0200, jouni.hogander@...kie.com wrote:
>>  net/core/net-sysfs.c | 21 ++++++++++++++++-----
>>  1 file changed, 16 insertions(+), 5 deletions(-)
>> 
>> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
>> index 865ba6ca16eb..72ecad583953 100644
>> --- a/net/core/net-sysfs.c
>> +++ b/net/core/net-sysfs.c
>> @@ -1626,6 +1626,12 @@ static void netdev_release(struct device *d)
>>  {
>>  	struct net_device *dev = to_net_dev(d);
>>  
>> +	/* Triggered by an error clean-up (put_device) during
>> +	 * initialization.
>> +	 */
>> +	if (dev->reg_state == NETREG_UNINITIALIZED)
>> +		return;
>> +
>
> Are you sure about this?  What about the memory involved here, what will
> free that?

In net/core/dev.c:free_netdev:

        /*  Compatibility with error handling in drivers */
	if (dev->reg_state == NETREG_UNINITIALIZED) {
		netdev_freemem(dev);
		return;
	}

I.e. driver is expected calling free_netdev in case of error and freeing
of device structure is done there. This brings up the question wether
put_device should be actually called in free_netdev also in error case?
Maybe that would be more correct as this free_netdev exists.

BR,

Jouni Högander