Skip to content

Conversation

@jonrebm
Copy link
Contributor

@jonrebm jonrebm commented Dec 14, 2025

Repeated call to ios->exit(ios); was observed as used after free using asan. Only call it once: In the exit handler.

Always call the exit handler and check there whether listenonly is set and there skip resetting the terminal if so.

Do not reset the terminal elsewhere.

Do not call the exit handler from the quit command, call exit instead.

@jonrebm jonrebm mentioned this pull request Dec 14, 2025
@jonrebm
Copy link
Contributor Author

jonrebm commented Jan 14, 2026

Has anyone taken a look at this one already? @ukleinek or @a3f maybe?

Repeated call to ios->exit(ios); was observed as used after free using
asan. Only call it once: In the exit handler.

Always call the exit handler and check there whether listenonly is set
and there skip resetting the terminal if so.

Do not reset the terminal elsewhere.

Do not call the exit handler from the quit command, just call exit,
invoking the signal handler too.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>

ios->exit(ios);
tcsetattr(STDIN_FILENO, TCSANOW, &sots);
if (listenonly)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be if (!listenonly) right? The terminal is configured for !listenonly so it should be restored in that case as well.

{
fflush(NULL);
microcom_exit(0);
exit(0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this is correct? You're commit message implies that exit(0) will somehow call microcom_exit() but when I tested this PR, that is not what happened.
Specifically exiting is not printed when stopping microcom with the quit command.

@jonrebm
Copy link
Contributor Author

jonrebm commented Feb 6, 2026

Thank you @michaelolbrich. Seems I need to run another round of testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants