Page MenuHomePhabricator

eldbus: Fix resource leak
ClosedPublic

Authored by devilhorns on Mar 14 2019, 10:14 AM.

Details

Summary

Coverity reports that we leak 'data' here (which can happen if we
error on 'eina_value_dup(value)'). Iniitalize 'data' to NULL, and add a
'free' call to cleanup 'data' before we return a rejected future.

Fixes Coverity CID1399097

@fix

Depends on D8350

Diff Detail

Repository
rEFL core/efl
Branch
devs/devilhorns/coverity
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 10380
devilhorns created this revision.Mar 14 2019, 10:14 AM
devilhorns requested review of this revision.Mar 14 2019, 10:14 AM
cedric requested changes to this revision.Mar 14 2019, 3:46 PM
cedric added inline comments.
src/lib/eldbus/eldbus_model_proxy.c
317

Hum, data should in that case be initialized to NULL. Also data might contain data that should be freed.

This revision now requires changes to proceed.Mar 14 2019, 3:46 PM
devilhorns added inline comments.Mar 15 2019, 5:20 AM
src/lib/eldbus/eldbus_model_proxy.c
317

Maybe it's because I have not had coffee yet, but I do not understand your disagreement here. If data is NULL, then free(data) will not cause any problems. If data contains something that should be freed then free(data) will do it....

devilhorns updated this revision to Diff 20617.Mar 15 2019, 5:55 AM
devilhorns edited the summary of this revision. (Show Details)

rebase

devilhorns updated this revision to Diff 20624.Mar 15 2019, 6:02 AM

no changes

devilhorns updated this revision to Diff 20639.Mar 15 2019, 9:42 AM
devilhorns edited the summary of this revision. (Show Details)

no changes

devilhorns updated this revision to Diff 20685.Mar 18 2019, 5:15 AM

no changes

bu5hm4n requested changes to this revision.Mar 18 2019, 8:08 AM
bu5hm4n added inline comments.
src/lib/eldbus/eldbus_model_proxy.c
317

I think the reason here is the goto in any line smaller than 294 will result in a free that passes in a invalid pointer (none initialized pointer)

This revision now requires changes to proceed.Mar 18 2019, 8:08 AM

So if we do Eldbus_Model_Proxy_Property_Set_Data *data = NULL; then this should be safe ... according to docs on free (man 3 free) "If ptr is NULL, no operation is performed".....

bu5hm4n accepted this revision.Mar 18 2019, 8:42 AM

I would say yes on that.

bu5hm4n requested changes to this revision.Mar 18 2019, 8:43 AM

Agree to the message not the revision yet :D

devilhorns updated this revision to Diff 20694.Mar 18 2019, 8:44 AM
devilhorns edited the summary of this revision. (Show Details)

Initialize data to NULL

cedric accepted this revision.Mar 18 2019, 9:57 AM

@bu5hm4n Can you approve this please ? I don't want to land it in a "needs review" state. Cedric already accepted, but it still says needs review .. I guess it is waiting on you

bu5hm4n resigned from this revision.Mar 19 2019, 5:54 AM

Quickfix

This revision is now accepted and ready to land.Mar 19 2019, 5:54 AM
devilhorns closed this revision.Mar 19 2019, 5:57 AM