Skip to content

Conversation

@fjpanag
Copy link
Contributor

@fjpanag fjpanag commented Jul 21, 2022

Reverts #6618

PR #6618 broke syslog, and it must be reverted (at least temporarily).

The buffer passed to syslog_default_write() is NOT NULL-terminated.
Printing depends on the value of buflen.

On the other hand, up_puts() needs the string to be NULL-terminated.

As a result, syslog over-runs the buffer and prints garbage.

The offending commit is specifically 8254ad9.

@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Jul 21, 2022

@fjpanag the new approach call up_nputs not up_puts, so it isn't a problem that the buffer isn't terminated by null pointer. Could you point out more specific place which assume the buffer is null terminated?

@fjpanag
Copy link
Contributor Author

fjpanag commented Jul 21, 2022

@xiaoxiang781216 in file syslog_channel.c in function syslog_default_write() there is a call to up_puts().

@pkarashchenko
Copy link
Contributor

pkarashchenko commented Jul 21, 2022

@fjpanag maybe better to replace that single call of up_puts() in syslog_default_write() with up_nputs() instead of reverting?

@xiaoxiang781216
Copy link
Contributor

@pkarashchenko @fjpanag please review patch #6661

@fjpanag
Copy link
Contributor Author

fjpanag commented Jul 21, 2022

Closing in favor of #6661.

@fjpanag fjpanag closed this Jul 21, 2022
@fjpanag fjpanag deleted the revert-6618-syslog branch July 21, 2022 12:54
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.

3 participants