search

raspberrypi / bookworm-feedback

14 stars 1 forks source link

sudo pcmanfm results in segfault #85

Closed theofficialgman closed 1 year ago

theofficialgman commented 1 year ago

copying from my initial post in the beta forum feedback thread https://forums.raspberrypi.com/viewtopic.php?t=353648&hilit=botspot#p2119974 . Refer to the post as to why the workarounds suggested are not suitable.

With sudo applications should fallback gracefully to using xwayland without issue (as they do in KDE and GNOME wayland). They do this by generating an xauthority file specific to the xwayland session on launch

KDE commit: https://invent.kde.org/plasma/kwin/-/commit/335d9c41925d9463a52f59ff61e7be6a84cae8c3 GNOME commits: https://gitlab.gnome.org/GNOME/mutter/-/commit/a8984a81c2e887623d69ec9989ae8a5025f7bd47 https://gitlab.gnome.org/GNOME/mutter/-/commit/eac227a203dba4d45398dfb85ec5b4610b5f3be7

From what I can gather this would need to be implemented into wlroots https://github.com/raspberrypi-ui/wlroots/tree/igalia-bookworm and not wayfire https://github.com/raspberrypi-ui/wayfire but correct me if I am wrong @spl237

spl237 commented 1 year ago

This is already implemented - an .Xauthority file in the user's home directory is generated at each boot by /etc/xdg/xwayauth.autostart, which is then used by any Xwayland processes spawned by wayfire.

spl237 commented 1 year ago

The location of the Xauth file is passed to Wayfire in an environment variable set in /usr/bin/wayfire-pi - if you are not seeing this working, I would guess it is because you are starting Wayfire with /usr/bin/wayfire and not the correct wrapper script.

theofficialgman commented 1 year ago

I'll investigate further later today and see if I can trace the root of the issue. What I assumed here may not be the cause of some applications failing to run under xwayland with sudo

theofficialgman commented 1 year ago

@spl237 found the bug. sorry it was not with wayfire. the specific issue that has been reported multiple times is with your pcmanfm fork when running with sudo under a wayland compositor it segfaults on launch.

the bug is right here: https://github.com/raspberrypi-ui/pcmanfm-bullseye/blob/master/src/pcmanfm.c#L209

XDG_SESSION_TYPE must not change when running the application as sudo which causes them to run under xwayland. so when you do so it still tries to run your use_wayland true guarded code and something there causes a segfault. removing that line entirely prevents the segfault but is not the solution of course. You will need to use another method to determine what window system the application is running under.

We can close this issue if you would rather have one made at pcman

My suggestion for a patch would be checking WAYLAND_DISPLAY variable presence (or some form of it)

if (getenv ("WAYLAND_DISPLAY")) use_wayland = TRUE;

That does work.

ghollingworth commented 1 year ago

Why are you trying to run pcmanfm as the root user, are you trying to log in as the root user and use a desktop environment? That’s not been supported in any Linux distribution for years.

theofficialgman commented 1 year ago

Why are you trying to run pcmanfm as the root user, are you trying to log in as the root user and use a desktop environment? That’s not been supported in any Linux distribution for years.

its extremely common use for users to want to run pcmanfm directly as root from terminal so that creating and editing files can be performed as root user. I should not need to provide any justification since any userspace application should not segfault when run as sudo.

ghollingworth commented 1 year ago

Best solution here is to switch back to X and use that.

spl237 commented 1 year ago

The standard mechanism to check for Wayland is to check XDG_SESSION_TYPE - see https://github.com/swaywm/sway/wiki/Running-programs-natively-under-wayland - this is what Qt does to check whether or not to use the Wayland backend.

There is no guarantee that WAYLAND_DISPLAY will be set - for example, Sway doesn't set it - https://unix.stackexchange.com/questions/748536/sway-is-not-setting-wayland-display

While I agree that the strcmp should be made safe against null strings, I am not convinced that checking WAYLAND_DISPLAY is a reliable (or even correct) way to check for running under Wayland. Checking XDG_SESSION_TYPE is a more reliable way of doing that.

spl237 commented 1 year ago

Fixed with https://github.com/raspberrypi-ui/pcmanfm-bullseye/commit/5b95fcbb92a78ca5af462da5e6040eebff56c053, which is a more reliable test for Wayland or X, as it doesn't rely on environment variables at all.