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

GUACAMOLE-1555: Call openlog() after client plugin unload #377

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

samiponkanen
Copy link

… This fixes segfaults from guacd_log() in cases where the client plugin has called openlog() with a non-NULL ident string.

… segfaults from guacd_log() in cases where the client plugin has called openlog() with a non-NULL ident string.
@mike-jumper
Copy link
Contributor

In what case does a plugin call openlog()? Why does a segfault occur if guacd does not re-call openlog()?

@samiponkanen
Copy link
Author

In what case does a plugin call openlog()? Why does a segfault occur if guacd does not re-call openlog()?

Hi Mike,

Vanilla guacamole-server does not call openlog() from plugin code.

In my use case I have integrated external code to protocols/rdp/channels/rdpdr/, and it is this external code that calls openlog(). This causes guacd to segfault everytime guacd_log() is called after the client plugin has been unloaded via dlclose() in libguac/client.c.

While debugging this issue I happened to find this:
https://bugs.freedesktop.org/attachment.cgi?bugid=24589&action=viewall

It is in no way related to guacamole-server, but it helped me to understand the problem and to verify the same pattern was the root cause for the guacd_log() segfault I was debugging.

Br,
Sami

@mike-jumper
Copy link
Contributor

I'm really on the fence regarding whether this is something that belongs in guacd. If third-party code invokes openlog() in such a way that it stomps on guacd's logging, I'd think that:

  • If possible, it shouldn't.
  • It would break guacd's logging while things are running, prior to the segfault in question.

Can you clarify what external code specifically needs to invoke openlog()?

@samiponkanen
Copy link
Author

I fully understand your concerns.

Another way to look at it: if a plugin does call openlog() and you start seeing seemingly unrelated segfaults in guacd_log(), then it really is not easy to figure out that the root cause is in the plugin code. The fix in this PR will remove the risk of getting those segfaults, even if the plugin code is not written like it should be.

But it's your call.

Br,
Sami

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