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

journal-gatewayd: fix segfault with certain request #3893

Merged
merged 1 commit into from
Aug 6, 2016

Conversation

eungjun-yi
Copy link
Contributor

@eungjun-yi eungjun-yi commented Aug 4, 2016

When client requests to get logs with follow and KEY=match that doesn't
match any log entry, journal-gatewayd segfaulted.

This fixes #3873.

@keszybz
Copy link
Member

keszybz commented Aug 5, 2016

This fixes the segfault, but I don't think that the correct solution. The failure only happens when follow is specified. With your patch an error would be returned, but what should happen instead is that we should return nothing and wait for matching entries to appear. (E.g. if the match is the MESSAGE_ID=fc2e22bc6ee647b6b90729ab34a250b1 and we are waiting for a coredump, we cannot assume that at least one will be available when we start. It's OK to get no entries initially with follow.)

@keszybz keszybz added bug 🐛 Programming errors, that need preferential fixing reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks journal-remote labels Aug 5, 2016
@eungjun-yi
Copy link
Contributor Author

@keszybz Okay, I'll retry it.

@@ -239,6 +239,10 @@ static ssize_t request_reader_entries(
m->size = (uint64_t) sz;
}

if (m->tmp == NULL && m->follow) {
return (ssize_t) 0;
}
Copy link
Member

Choose a reason for hiding this comment

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

Please don't use {} around a single statement (see CODING_STYLE).

The cast is not needed, 0 is already a signed integer, and the compiler will do the size promotion automatically if necessary.

Copy link
Member

@keszybz keszybz Aug 6, 2016

Choose a reason for hiding this comment

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

Please add 'Fixes #3873' to the commit description so that github will close the issue automaticallly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for review. I've updated it.

When client requests to get logs with `follow` and `KEY=match` that
doesn't match any log entry, journal-gatewayd segfaulted.

Make request_reader_entries to return zero in such case to wait for
matching entries.

This fixes systemd#3873.
@keszybz keszybz removed the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Aug 6, 2016
@keszybz keszybz merged commit 3475fc5 into systemd:master Aug 6, 2016
@keszybz
Copy link
Member

keszybz commented Aug 6, 2016

Thanks!

edolstra pushed a commit to NixOS/systemd that referenced this pull request Sep 30, 2016
When client requests to get logs with `follow` and `KEY=match` that
doesn't match any log entry, journal-gatewayd segfaulted.

Make request_reader_entries to return zero in such case to wait for
matching entries.

This fixes systemd#3873.
(cherry picked from commit 3475fc5)
AnchorCat pushed a commit to AnchorCat/systemd that referenced this pull request May 9, 2017
When client requests to get logs with `follow` and `KEY=match` that
doesn't match any log entry, journal-gatewayd segfaulted.

Make request_reader_entries to return zero in such case to wait for
matching entries.

This fixes systemd#3873.
(cherry picked from commit 3475fc5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Programming errors, that need preferential fixing journal-remote
Development

Successfully merging this pull request may close these issues.

systemd-journal-gatewayd crashes if request has follow and unmatched KEY=match parameter
2 participants