netdev - Re: [PATCH 4/5] ISDN-Gigaset: Release memory in gigaset

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <e2f7c915-45f0-0716-4a58-2da9a79a0335@users.sourceforge.net>
Date:   Tue, 27 Sep 2016 17:10:23 +0200
From:   SF Markus Elfring <elfring@...rs.sourceforge.net>
To:     Paul Bolle <pebolle@...cali.nl>
Cc:     gigaset307x-common@...ts.sourceforge.net, netdev@...r.kernel.org,
        Karsten Keil <isdn@...ux-pingi.de>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors@...r.kernel.org,
        Julia Lawall <julia.lawall@...6.fr>,
        Tilman Schmidt <tilman@...p.cc>
Subject: Re: [PATCH 4/5] ISDN-Gigaset: Release memory in gigaset_initcs()
 after an allocation failure

>> @@ -772,8 +775,9 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
>>  
>>  	gig_dbg(DEBUG_INIT, "cs initialized");
>>  	return cs;
>> -
>> -error:
>> +free_bcs:
>> +	kfree(cs->bcs);
>> +report_failure:
>>  	gig_dbg(DEBUG_INIT, "failed");
>>  	gigaset_freecs(cs);
> 
> gigaset_freecs() is not a function I look at for the fun of it. But
> still, in it we find:
> 
> 	case 0: /* error in basic setup */
> 		[...]
> 		kfree(cs->inbuf);
> 		kfree(cs->bcs);
> 
> As far as I can tell we will call those two kfree()'s if we jump to
> "error". So, contrary to your analysis, I don't think we leak cs->bcs.

You are right.

Thanks that you pointed this source code place out again.

I imagined that the exception handling implementation could be more direct
somehow for a while. But this function takes extra care for data synchronisation
by a mutex.

Now I recognise also that this proposed update step "4" was inappropriate.
I'm sorry for the confusion I introduced here.

Regards,
Markus