Change in ...libosmocore[master]: fix: vty crash by logging to killed telnet session

neels gerrit-no-reply at lists.osmocom.org
Tue Aug 27 15:40:57 UTC 2019


neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/15265 )

Change subject: fix: vty crash by logging to killed telnet session
......................................................................


Patch Set 1:

(4 comments)

https://gerrit.osmocom.org/#/c/15265/1//COMMIT_MSG 
Commit Message:

https://gerrit.osmocom.org/#/c/15265/1//COMMIT_MSG@11 
PS1, Line 11: segfaults: the vty->obuf is already NULL.
> Why do we try to send a message to a killed session first of all? That seems to be the route of the  […]
I want to make sure that we cannot ever crash by writing to a NULL ofd. Whatever the reasons for the NULL ofd, and whatever the reasons for wanting to log to it.

Fixing the higher level fail would be: close down the logging target before taking down the ofd. I care about that only secondarily. This is only one cause for logging to NULL that we found now.

Fair enough, if we ensure closed telnets always close down log targets safely the issue would not exist, but I am not willing to verify that now. That is why I want these NULL checks at the buffer API level. It makes all other calling code bugs non-fatal in a trivial way, without the need to fully audit the ancient vty code.

Can we please get over this and not make a huge thing of it?


https://gerrit.osmocom.org/#/c/15265/1//COMMIT_MSG@16 
PS1, Line 16: Also guard all buffer_*() functions against a NULL buffer argument, which
> this looks like a hack to fix other parts of the code which may be wrong, and looks like it should b […]
If the calling code paths are not trivially clear, it is good practise to guard against NULL pointers. Rather drop a log line than crashing the entire program.


https://gerrit.osmocom.org/#/c/15265/1//COMMIT_MSG@20 
PS1, Line 20: Related: OS#4164
> This ticket doesn't look related to me.
this one is correct, the one in the comment has a typo


https://gerrit.osmocom.org/#/c/15265/1/src/vty/vty.c 
File src/vty/vty.c:

https://gerrit.osmocom.org/#/c/15265/1/src/vty/vty.c@266 
PS1, Line 266: 			 * of this same (killed) telnet session. See OS#4146. */
> OS#4146 (https://osmocom. […]
aha, the commit log has the correct one: #4164. Here there is a number swap. thx



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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Idca3f54dc986abf6784790c12e69e02bdf77cb41
Gerrit-Change-Number: 15265
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-CC: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Tue, 27 Aug 2019 15:40:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin at sysmocom.de>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190827/d02cd0f4/attachment.html>


More information about the gerrit-log mailing list