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 <ssperl...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Stefan Sperling <ssperl...@sysmocom.de> Gerrit-Comment-Date: Wed, 23 May 2018 13:44:55 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No
