Change in libosmo-abis[master]: fix double-free/use-after-free of pointers in struct e1inp_line

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Stefan Sperling gerrit-no-reply at lists.osmocom.org
Wed May 23 13:44:55 UTC 2018


Stefan Sperling has posted comments on this change. ( https://gerrit.osmocom.org/9252 )

Change subject: fix double-free/use-after-free of pointers in struct e1inp_line
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/9252/1/src/e1_input.c
File src/e1_input.c:

https://gerrit.osmocom.org/#/c/9252/1/src/e1_input.c@429
PS1, Line 429: 		/* Remove our counter group from the global counter list
> I have the feeling this code is wrong. […]
These parts of the talloc API are not very intuitive.

As far as I undestand it, references obtained with talloc_reference() are counted in addition to the original allocation. Which means their count drops to zero, not to 1, before the last talloc_free().

We use talloc_unlink for the rate counters because we want to dereference the last remaining parent context (line) pointer to free the counters. So we remove our reference and check if we were last, then deref the parent context pointer (line) to free line->rate_ctr, then free the parent context. There might be other ways of doing this, but this one works, and the other ways aren't any more intuitive than this one.

Additional references to line->driver_data are implicitly handled by talloc_free().
We don't care which parent context's reference is removed in that case. The last talloc_free() occurs with a reference count of 0 and will free driver_data.

Maybe we should add a comment to clarify this, but on the other hand it is not unreasonable to expect readers of the code to understand the nuances of the talloc API.



-- 
To view, visit https://gerrit.osmocom.org/9252
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f4724b4a5a064801591e9acf4f2fd1db006d082
Gerrit-Change-Number: 9252
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Comment-Date: Wed, 23 May 2018 13:44:55 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180523/9b49f8b2/attachment.htm>


More information about the gerrit-log mailing list