Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mmgrok looks like it is copying the msg data into the "instance" structure erroneously #1359

Open
portante opened this issue Jan 13, 2017 · 2 comments

Comments

@portante
Copy link
Contributor

Seems like mmgrok is not working quite like the way we'd want it to work.

At line https://github.com/rsyslog/rsyslog/blob/master/contrib/mmgrok/mmgrok.c#L163, it seems that the instanceData data structure pszSource field is initialized with the configured value, e.g. msg (one could reference a JSON property, like "!myfield", just as well).

However, as we process messages, it appears that one line https://github.com/rsyslog/rsyslog/blob/master/contrib/mmgrok/mmgrok.c#L351, we overwrite the instanceData structure's pszSource field with the actual message buffer data.

Maybe I am reading this wrong, but why does it just pass that data as a parameter instead?

@rgerhards
Copy link
Member

I am not sure if the contributor is following the github issue trackers. It might be better to mail him directly.

@rgerhards rgerhards added this to the v8.33 milestone Nov 26, 2017
@rgerhards rgerhards modified the milestones: v8.33, v8.34 Feb 15, 2018
@rgerhards rgerhards modified the milestones: v8.34, v8.35 Apr 3, 2018
@rgerhards
Copy link
Member

@portante Is there still interest in this issue tracker? I just had a look at it and I agree that it is wrong. I also have seen that strtok() is used, which is not thread-safe. So the module will potentially make rsyslog segfault.

Given the fact that besides this tracker here we never received any attention regarding mmgrok, I wonder it if it really is useful to try to make it behave differently (except for strtok() for reliability reasons).

rgerhards added a commit to rgerhards/rsyslog that referenced this issue May 4, 2018
The modules used strtok(), which is not thread-safe. So it will potentially
segfault when multiple instances are spawned (what e.g. happens on busy
systems).

This patch replaces strtok() with its thread-safe counterpart
strtok_r().

see also rsyslog#1359
@rgerhards rgerhards removed this from the v8.35 milestone May 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants