I told you so, 2021 edition

Cinnamon-screensaver got popped, again.

If you are not running XScreenSaver on Linux, then it is safe to assume that your screen does not lock.

The latest:

Previously:

You will recall that in 2004, which is now seventeen years ago, I wrote a document explaining why I made the design trade-offs that I did in XScreenSaver, and in that document I predicted this exact bug as my example of, "this is what will happen if you don't do it this way."

And they went and made that happen.

Repeatedly.

Every time this bug is re-introduced, someone pipes up and says something like, "So what, it was a bug, they've fixed it." That's really missing the point. The point is not that such a bug existed, but that such a bug was even possible. The real bug here is that the design of the system even permits this class of bug. It is unconscionable that someone designing a critical piece of security infrastructure would design the system in such a way that it does not fail safe.

Especially when I have given them nearly 30 years of prior art demonstrating how to do it right, and a two-decades-old document clearly explaining What Not To Do that coincidentally used this very bug as its illustrative strawman!

These bugs are a shameful embarrassment of design -- as opposed to merely bad code.

This same bug keeps cropping up in these other screen lockers for several reasons.

  1. Writing security-critical code is hard. Most people can't do it.

  2. Locking and authentication is an OS-level problem. And while X11 is at the heart of the OS of a Linux desktop computer, it was designed with no security to speak of, and so lockers have to run as normal, unprivileged, user-level applications. That makes the problem even harder.

  3. This mistake of the X11 architecture can never, ever be fixed. X11 is too old, too ossified, and has too many quagmire-trapped stakeholders to ever make any meaningful changes to it again. That's why people keep trying to replace X11 -- and failing, because it's too entrenched.

As always, these bugs are terrible because bad security is worse than no security. If you knew for a fact that your screen didn't lock, you would behave appropriately. Maybe you'd log out when you walked away. Maybe you wouldn't use that computer for certain things. But a security placebo makes you behave as if it's secure when in fact it is not.

One of the infuriating parts of these recurring bugs is that the screen-locker part of XScreenSaver isn't even the fun part! I do not enjoy working on it. I never have. I added it in response to demand and necessity, not because it sounded like a good time. I started and continue this project as an outlet for making art. I'd much rather be spending my time pushing triangles.

Sigh.

And in not-at-all-unrelated news:

Just to add insult to injury, it has recently come to my attention that not only are Gnome-screensaver, Mate-screensaver and Cinnamon-screensaver buggy and insecure dumpster fires, but they are also in violation of my license and infringing my copyright.

XScreenSaver was released under the BSD license, one of the oldest and most permissive of the free software licenses. It turns out, the Gnome-screensaver authors copied large parts of XScreenSaver into their program, removed the BSD license and slapped a GPL license on my code instead -- and also removed my name. Rude.

If they had asked me, "can you dual-license this code", I might have said yes. If they had asked, "can we strip your name off and credit your work as (C) William Jon McCann instead"... probably not.

Mate-screensaver and Cinnamon-screensaver, being forks and descendants of Gnome-screensaver, have inherited this license violation and continue to perpetuate it. Every Linux distro is shipping this copyright- and license-infringing code.

I eagerly await hearing how they're going to make this right.

Previously, previously, previously, previously, previously.

Tags: , , , , , ,

101 Responses:

  1. different Jamie says:

    A copyright letter to Big RedBlue could be entertaining.

    Or on the other hand, not.

  2. Zygo says:

    There's an update in Ubuntu 20.04 (Mint 20.x) which backports xorg 1.20.9 towards focal... that backport actually messed up and missed the CVE fix (which is present in the groovy version), so upgrading to this temporarily fixes this issue. When Ubuntu realize this mistake they'll backport the CVE fix again and this will break again.

    I am familiar with this process! If upstream releases a regression and then a fix in the next point release, there will be Ubuntu, releasing the same regression some weeks after both the regression and fix are known. Then the package maintainer takes a vacation so the fix finally lands on user machines months later.

  3. jer says:

    This prompted people to start digging into other alternative screen lockers, like mate-screensaver: Screen locked partially with "Lock-screen" #183. I recall having seen reports of the external-monitor-switch-during-sleep approach before in other screen locker(s?).

    • jwz says:

      To my shame, XScreenSaver once suffered a bug where hot-swapping monitors while the display was locked could, under some circumstances, cause a crash.

      The RANDR stuff is a very good example of a library that is pretty complicated and hard to test, and where I really, really wish that it was not linked into the xscreensaver daemon at all. However, the consequences of not including it are even worse: without handling monitor hot-swaps, plugging in an external monitor while locked would expose a new, visible desktop. You wouldn't be able to type on it, but you'd be able to see it, and that's still a security problem.

      The xscreensaver daemon is as small as I could make it, but it's still far larger than I'm comfortable with.

      In summary, everything is terrible.

      • pde says:

        Was the RANDR stuff terrible enough that you'd consider putting it in a separate process?

        • jwz says:

          I'm not sure it can be -- screen reconfiguration has to be processed while grabs are held. Anyway, the source of that particular bug was a weird interaction between monitor hot-swaps and PAM-related SIGCHLD handlers.

          • pde says:

            Random not necessarily helpful feedback. I tried xscreensaver as a gnome-screensaver replacement after reading this post. Things that went wrong with Debian's 5.45+dfsg1-1

            * the RANDR stuff! Even with no plugging or unplugging monitors, content on an external monitor remains visible while the screen is locked.
            * I spent a while trying to figure out while the hideous zoom video client kept re-launching itself after I closed it. Eventually I traced it to the auto-generated .xscreensaver file, which was trying to launch "zoom -root"(if that binary was ever in the debian packages, it isn't anymore).

      • aaa.bbb.ccc.ddd says:

        Security in the presence of hot-plugged monitors might be made more robust by disabling the udev rules for monitor hot-plug while the screen is locked.

        • jwz says:

          Your laptop is plugged into an external monitor with the lid closed. Since the laptop's built-in monitor is off, the system is currently configured to have one active monitor, the external one, so that's where the unlock dialog is going to appear. The screen locks. You pull the plug and walk away. Later, you open the laptop lid.

          So now your screen is locked, which means that you are ignoring monitor hot-swaps. That means that you're rendering the unlock dialog onto a monitor that no longer exists, and rendering nothing onto the monitor that does.

          Also udev is Linux-specific.

    • Alexei Sorokin says:

      Because the bug only occures on GTK 3.22+, the number of distributions affected was small at the time, so every instance of the bug in the wild could be pretty much tracked.
      I had hoped that this could be resolved without serious implications.

      Not how it worked out in practice: Ubuntu 18.04 refused to update hard.
      Even after publishing a CVE and getting broad attention to the bug.

      Which was futile in the end as Ubuntu 18.04 is affected to this day.
      But at least people know that it is.

      I sat on this code for a year before publishing and it still contained a serious flaw. I am sorry for that.

  4. Sean says:

    I enjoy that the commit message removing you from AUTHORS says 'not much left of that'. Not 'there is none of this code left that is under a different license', just 'not much', so we're cool, right?

    Even if there was none of your code left (and ignoring the license violation), it would still be polite to at least mention that it was based on XScreenSaver originally.

    • rjc says:

      @Sean - having read the linked commit message as well as the AUTHORS file, it is pretty clear that they are referring to the content of said file - one line with their own name - not the amount of jwz's code left in the codebase.

      Let's not read into things that aren't there :^)

      • Ben says:

        The guy is clearly a tool, it's totally reasonable to read things into it that are there.

        https://twitter.com/jonmccann/status/1345237884283088897

        • Angry Cyberman says:

          Man, what a prick

          "Dude, we two has-beens debating long obsolete software is a sad use of everyone’s time. Happy new year. And kids, remember: never meet your heroes."

          jwz should sue this guy for copyright infringement just to put him in his place IMO

          I guess that spending a 5 figure sum on lawyers just to shut down some troll noob might not be the best use of time and money though..

    • "I have destroyed most of the evidence of my crime." Now you've done two crimes.

    • WorBlux says:

      It's the old ship of Theseus problem. Even if you have replaced all of the code it's in a sense the ship. Even if you had replaces all of code, existing code is enough of a derivative work of the old code that there is likely an enforceable copyright claim there.

      • FeepingCreature says:

        I don't think derivative work is transitive in that way. I think when you file a copyright suit, the state of the original and new code is taken in isolation - access to the original work may be used to establish that copying took place, but I don't think it matters legally as long as none of the original code remains. Compare a videogame where you use placeholder art from another game during development; compare works like 50 Shades that started as fanfic and were scrubbed of identifiable elements in editing.

  5. k says:

    Thanks you allmighty jwz, I am jumping out of joy same as you (really, hillariuos) and swear an oath to use only xscreensaver ever.

  6. x64k says:

    The bug ticket, if you haven't read it, is very much "2021 Linux in a nutshell", too.

    You'd think this bug was found after a security audit or something. A brainstorming session, then someone looked at the source code looking for bugs. Or maybe some cyberpunk neo-noir hacker sat at his keyboard for 42 hours straight looking at green letters on a black background until he found the dreaded NULL pointer and he attained enlightenment by staring into its void.

    Nope:

    > A few weeks ago, my kids wanted to hack my linux desktop, so they typed and clicked everywhere, while I was standing behind them looking at them play... when the screensaver core dumped and they actually hacked their way in! wow, those little hackers...

    • phuzz says:

      "Non-automated fuzz-testing using child labour"

    • annoynymous says:

      My friend's cat managed to do it, too, using a touchscreen.

    • Tedd says:

      "green letters on a black background"

      Look where you're posting.

    • not pj says:

      I vaguely recall similar thing happened when I was living in a dorm - except it wasn't kids, but a fellow student that somehow managed to crashed his colleague's lock screen. He promptly set a crudely-drawn penis as his wallpaper.

      I was using xscreensaver at the time as well, so my screen was dick-free.

  7. Soni says:

    uh so like what about using something like lunatic to run the toolkit in-process with fail-safe guarantees? is that something to look into or would it be a waste of time? .-. (really want to do it tho)

  8. tfb says:

    The 'this class of bug should not be possible' argument is great. But it's a cause that was lost before most people working on Linux were born. The class of bug which is 'buffer overflows' (which, I bet, is what is underlying the bugs here) can be (almost entirely) prevented by language design. Such languages have existed almost since there were high-level languages. Yet because it was hard to make them fast and small enough to be really usable on a PDP-11 fifty years ago we all live with buffer overflows, except now we live with buffer overflows in systems vast beyond comprehension.

    And we all know this and still we pretend there is hope.

    • jwz says:

      Oh, I'm aware. But, one can still code defensively. One can approach the problem with the assumption that buffer overflows are out there, and are going to happen, and work to minimize the attack surface.

      One can. Most don't.

      • tfb says:

        Oh, sorry, I knew you knew: I was just feeling more doomed than usual, if that's possible.

        I think that 'most don't doesn't capture the problem': most can't: for almost all humans it is beyond their ability to write large, safe programs in a language which does not enforce safety.

        (And someone is now going to say that enforcing safety is a halting problem and argue that because it is we should not even bother solving the bits we can easily solve. The curse of the one-bit mind is neverending.)

  9. BHN says:

    Sue them for copyright infringement to teach them a lesson.

    Use any money left after paying the lawyer to pay some of the DNA's bills.

    (I realize that's several universes over from this one, but it's what should happen.)

  10. yossarian says:

    I'am using Linux Mint Cinnamon and your "XScreenSaver" is not installed by default, and if I want to install it there is link to your web page "https://www.jwz.org/xscreensaver/"

    • says:

      I believe the claim is that the cinnamon-screensaver that is presumably installed by default is based on xscreensaver code and still contains parts of that code. It's immaterial whether there is another packet that is licensed properly.

  11. Laurent Pointecouteau says:

    "That's why people keep trying to replace X11 -- and failing, because it's too entrenched."

    Are they failing though? For what I understand, the only thing preventing Ubuntu to set up Wayland as default is lack of support from the proprietary Nvidia driver. It's a huge hurdle, sure, but I just wanted to stress that your concerns have been those of the Wayland developers since the beginning, and that a lot of distros can run Wayland without even relying on X for many years. For this reason, I doubt that "If you are not running XScreenSaver on Linux, then it is safe to assume that your screen does not lock" is a valid statement anymore.

    Also, gnome-screensaver development has stopped four years ago, the Gitlab project has been archived and nobody can ever commit to it again. Some distros rely on outdated software by design. To my opinion, this is the kind of thinking that is too entrenched, and harming the work of well-meaning developers that are still trying to do useful things with the Linux desktop.

    (yeah I have a friend who works on GNOME and I think that's a cool desktop, but I also enjoy the xscreensaver hacks a lot so if I shall die on that hill, ainsi soit-il.)

    • jwz says:

      Me: People keep talking about replacing X11, but nobody has done it.
      You: But someone might be planning to replace X11, real soon now.

      We've been hearing that for, uh, quite some time. So yes. They have so far failed.

      Gnome-screensaver might be "officially obsolete" but mate-screensaver and cinnamon-screensaver, which are the same thing, are still in active use. And all three of them are still being shipped by distros.

      • Kealper says:

        I haven't checked mate-screensaver, but the cinnamon-screensaver of today isn't the old, defunct gnome-screensaver of yesteryear. Back in 2016, cinnamon-screensaver was rewritten in Python, the commit for that rewrite is here.

        The root cause of this seems to be a libcaribou one which the Mint team has now patched, though parts of your post about security-related things and the need to fail-safe are very valid.

        • Kealper says:

          Quick correction to my above reply: I said libcaribou, I meant to say libcairo, one of the components of caribou, sorry about that.

        • jwz says:

          In fact, all of my post is valid.

          Cinnamon-screensaver has indeed rewritten much of the C code in Python, but it still includes C code written by me that it inherited from Gnome-screensaver, and that has had my name, copyright and license stripped off.

          There is also some of my code in Cinnamon-screensaver that is correctly attributed, but the violations are very much still present.

          (And that means that they are mixing GPL and BSD code in the same executable, which is A) at best unwise but also B) not my problem or concern.)

          • Kealper says:

            I stand corrected, I didn't trawl through the whole codebase, I just seen that they had done a rewrite and incorrectly assumed that all of the old C-based code was removed, my apologies.

    • popey says:

      It's not just hardware support that made us choose to revert back from Wayland to X11. It was also that if the shell crashed, it took out all the user applications. For all its faults, under X11, if the shell crashes, and restarts, your windows are (typically) still there. Maybe we'll look at Wayland again for the next LTS in 2022, which means it'll have to be default in late 2021 (for Ubuntu 21.10) as a test bed. Fingers crossed the driver support you mention is ready by then too.

    • Cowmix says:

      The funny part is the Nvidia driver for X11 on Ubuntu is a dumpster fire itself. I'm running that configuration right now and let me tell you, it sucks horribly. Specifically, my ENTIRE desktop locks up all the time.

    • abarbarian says:

      "That's why people keep trying to replace X11 -- and failing, because it's too entrenched."

      Are they failing though? For what I understand, the only thing preventing Ubuntu to set up Wayland as default is lack of support from the proprietary Nvidia driver.

      Nah the real reason is that Window Maker does not work with Wayland yet ;-)

      • x64k says:

        > Nah the real reason is that Window Maker does not work with Wayland yet ;-)

        Hush, brother, they must not know of the one true window manager!

  12. NdrU says:

    Datapoint of one, but I've been running on wayland for over a year now, without any significant issues.

    Some applications are still running using XWayland, but most are now native.

    Funnily enough, the screen locker that I use has a very similar issue that you mentioned above reported recently: https://github.com/swaywm/swaylock/issues/162

    • jwz says:

      So I guess Wayland also treats locking and authentication as a userspace problem, not an OS problem. Good to hear that Wayland's design is bug-compatible with X11's!

      • justJanne says:

        Well, wayland treats everything as a userspace problem, because it lets the compositor handle everything (even stuff that previously belonged to the XServer).

        But it (and associated projects) also make it easier to handle locking at a much higher level, ensuring that locking isn't just a fullscreen window (which is, even if your software might implement it correctly, extremely fragile), but instead is actually just a boolean defining whether to show an actual session or a screen locker on that tty (and then the screen locker is responsible for communicating with the compositor to change this setting).

        Obviously some compositors implement that in the X11 style, and bring all the same issues back.

      • jf says:

        So the real issue here is that you want to ensure access to all graphics/input devices is revoked until something else happens, from the perspective of the OS it doesn't really matter where you put this policy as long as it's above the unprivileged user layer. (In modern systems, the X server sort of runs at this layer)

        Sway doesn't do it right, but in GNOME's wayland implementation and some others, the secure solution there is to put the screen locker in the display server. In theory this is better because the display server basically overrides the whole input stack while the screen locker is active. If you manage to crash the screen locker you also crash the display server, so that at least gives you some level of fail-closed behavior.

        The other way to do it would be to put some logic in the session and display manager to forcibly switch to another VT when the lock happens. The display manager would monitor the VT and try to keep restarting the locker if it crashes. In theory that would work for both Wayland and X11 and would be the most secure, but it hasn't happened yet.

        • Robert Buj says:

          I don't mind the screen lock being handled by the display manager, there are even people who click to switch user to lock the screen ;-)

  13. I just checked, and my system apparently has slock set up, not that I've ever actually used it in a scenario where I needed real security. Chances are slock is damned in some way, no different than most of the other garbage Suckless produces in the misguided pursuit of simplicity under UNIX.

    The real bug here is that the design of the system even permits this class of bug.

    Giving it more thought, the idea that a program can take over every screen and all input is also fundamentally misguided. This is exactly the manner of thing a system program should be able to do, which a normal program shouldn't, but the UNIX solution is to provide nothing instead.

    I've never actually written an X11 program. UNIX, in its pursuit of simplicity, has made it too damn difficult for me to ever consider drawing to the screen, because the ability is layered underneath millions of lines of broken interfaces. I'd to settle for writing terminal libraries for myself instead, because Ncurses is grotesque as well, but that just means getting to learn how xterm doesn't follow the standards and how the idiots butchered a halfway decent design when they extended it.

    I find it interesting, jwz, how these operating systems are tens of millions of lines of code, and yet do so little that's reusable in any way. I want to add a recollection system to a program of mine, to have undo and redo functionality, and I have to design it myself; every little thing I want I tend to need to design myself, because the options either don't exist, require me to pull in code thousands of times larger than the entirety of the program I'm writing, or the provided option is simply broken in addition to this.

    I've come to view conventional computer security as an illusion, no different than IP addresses; the future of security isn't locking the machine or providing passwords, but using real cryptography to gate access to resources.

    It's possible for applications to tell "real" and "synthetic" events apart, and it's not unusual for toolkits to ignore synthetic events, as a security measure.

    That's beautiful. I've designed ugly little systems which aren't sandboxed and abstracted properly like this, but the difference is I abandoned them, instead of doing what UNIX fanatics do and building an altar of worship instead.

    I've written about the value of entirely rewriting software, so I'd disagree that rewriting XScreenSaver is a waste of time, but my thinking about this regards the original author doing so, and it's unfortunate these others seem incompetent.

    I'll stop my ranting here.

    • Brian Reno says:

      Chances are slock is damned in some way, no different than most of the other garbage Suckless produces in the misguided pursuit of simplicity under UNIX.

      If you want to be secure against a new external monitor being added, put epoxy in your unused video ports.

    • Martin Uecker says:

      It is fairly simple to create a window and draw on it with X11. You can do it in a couple of lines of code. I like XCB which is a bit more low-level and Xlib and I actually like the X protocol. It is well designed and quite nice. Nobody forces one to use all the GUI libraries on top of it.

      I also see no fundamental reason why on could add built-in support for secure screen locking to X, but I haven't thought about it much.

      • jwz says:

        There is no fundamental reason that locking couldn't be built into the X server itself. And it's a good idea, because that would mean that a crash in the locker would result in the user being logged out, which, though annoying, is safe.

        Making this happen would "only" require:

        • Writing the code to put a localized GUI inside an X server extension (something that no other extension has done).
        • Writing the code to get the X server to talk to PAM and/or other authentication libraries.
        • Getting that code approved by and packaged by the Xorg bureaucracy.
        • Getting that code approved by and packaged by two dozen Linux and BSD distros.
        • Modifying or replacing XScreenSaver to use that server extension instead of locking on its own.
        • Waiting 3 years for Debian to actually distribute it.
        • Fielding the complaints that the server's built-in locking GUI does not match the desktop theme's colors and fonts.
        • Fielding the complaints that the server's built-in locking GUI doesn't support the use of user-space input methods and on-screen keyboards that require 30 different libraries and full window management to work.

        See? Easy!

        • Martin Uecker says:

          I did not suggest to put the GUI code for the screen server into X. The idea would be to add a special API for the screensaver to use. If the screen saver crashes, it would not unlock the screen and the X server would prevent any further access (or maybe could be unlocked using some other fall-back method).

          • jwz says:

            Well, no, that plan doesn't work. It's the same as the grab problem.

            And also has all of the same "get a new extension approved and distributed" problems.

            • Martin Uecker says:

              Can you explain why this could not be solved by a specialized API the X server provides?

              "get a new extensions approved and distributed" would certainly take time and effort.

              • jwz says:

                I did, in the linked document.

                Look, if you think you have a design that will work, write it up. This is not the forum for workshopping your vague ideas.

        • jafd says:

          > Making this happen would "only" require:

          ...taking all the good bits off XScreenSaver, of course, and carefully modeling them in ;-)

          (and giving proper credit too. I thought BSD licenses were easy enough to comply with for even a lobotomized turtle, got surprised someone managed to fail at that at all.)

          Oh, except that bit:

          > • Getting that code approved by and packaged by the Xorg bureaucracy.

          They say Xorg is practically unmaintained now (how typical: the new thing barely works, but they are happily abandoning the old thing already), so I guess it's a free-for-all now. Just package it as RPM and DEB, and tell people where to find it. Fuck bureaucracies.

          > • Writing the code to put a localized GUI inside an X server extension (something that no other extension has done).

          The NVidia's blob driver has a hutzpah to shove their fullscreen logo into your eyes on Xorg startup, so I guess it can be done somehow.

  14. Ham Monger says:

    Slackware still ships xscreensaver by name (and with your name on it). It also ships other stuff that pretends to lock the screen, but not {gnome,mint,cinnamon}-screensaver. For what that's worth.

    On the other hand, the out-of-memory killer on my system seems to take glee in killing xscreensaver-daemon, so that's delightful.

    • jwz says:

      That is indeed delightful. XScreenSaver attempts to mitigate that by writing -1000 to /proc/$$/oom_score_adj, but that might only work if it's setuid. See #ifdef HAVE_PROC_OOM.

      Also: is the XScreenSaver daemon actually large compared to other processes? Because it should stay pretty small.

      • Torkell says:

        I'm convinced that the oom-killer's logic is to find the process that would be the most hilariously inconvenient to lose and kill that, rather than the process that's actually responsible for running out of RAM. The best example I've seen of this is on a server where snmpd went berserk and chewed through RAM, so the oom-killer triggered and decided to kill off cron instead (which we didn't discover until several months later when /var/log ran out of space as logrotate wasn't being run).

        • Wolf says:

          This is the main reason I disable the OOM killer entirely along with memory overcommit in any mission-critical servers at this point. It's a game of Russian Roulette otherwise.

        • Richard Barrell says:

          I'm sorry for your loss, but that is really funny. The hilaroty optimisation algorithm worked perfectly.

          (My other favourite thing is randomly losing sshd.)

      • Wolf says:

        Checking Slackware's latest package for xscreensaver (5.45 for Slackware 14.2), there's no instance of 'oom' as a case-insensitive search across the entire contents of the .tar.xz when unpacked, not in any metadata, scripts, binaries, or anything else. So looks like they compiled it with that option disabled, lovely. :S

      • Ham Monger says:

        I'll grep dmesg next time I notice it's been whacked and report back. My hunch is that a saver module is getting out of hand and somehow that gets attributed to the wrong process. The daemon should be setuid properly but I'll confirm that too.

      • rworkman says:

        Slackware ships xscreensaver setgid shadow instead of setuid root; that's likely why it occurs :/

        • Ham Monger says:

          In Slackware-current, it's not even setgid any more, presumably due to the migration from shadow to PAM. It sounds like building it setuid might be a win though.

          As rworkman pointed out, in Slackware 14.2, it is indeed not setuid, but setgid, and I'm pretty sure setgid isn't enough to allow suppressing the OOM killer. If you still want the gory details of why the OOM choose xscreensaver to clobber, I'll have to get back to you the next time it gets whacked.

          (Now that the evidence is staring at me, I suppose I must have installed it setuid for some new-at-the-time saver long ago, and when I went to -current, I used the new and non-setuid package. I apologize for the confusion.)

          • jwz says:

            If you still want the gory details of why the OOM choose xscreensaver to clobber, I'll have to get back to you the next time it gets whacked.

            Well, I'm certainly curious if you can figure out why it chose to whack XScreenSaver. Though chances are you won't be able to tell, and even if you could, chances are I couldn't do anything about it.

            Hooray!

          • jwz says:

            This says:

            So the ideal candidate for liquidation is a recently started, non privileged process which together with its children uses lots of memory, has been nice'd, and does no raw I/O.

            No wonder it keeps picking XScreenSaver! It also says:

            We add the vmsize of the childs if they have an own mm.

            I don't know what an "mm" is, but maybe there's some sysctl nonsense I could do to make it not count the size of the screenhacks when deciding whether to assassinate the daemon?

            I also can't tell which of these would be more effective, or if they are the same:

            echo -1000 > /proc/$$/oom_score_adj
            echo -17 > /proc/$$/oom_adj

            • exec says:

              >I don't know what an "mm" is

              Memory manager?

              • chaosite says:

                It refers to the "mm" field in the process's "task_struct". That struct holds all the memory mappings that process has.

                I think that "childs that do not have an own mm" are threads? Just guessing here.

            • chaosite says:

              I just read the code.

              -1000 to oom_score_adj is the same as -17 to oom_adj and both should make the OOM killer leave those processes alone.

      • pakraticus says:

        With cgroups and virtual memory auditing the OOM killer will start with the cgroup with the process that made the offending memory request before it kills other things.
        We had chip designers killing their Xsessions when they looked at huge waveforms. We fixed it with manual cgroups. A kernel command line option to enable virtual memory auditing. Then three lines of clever in the system wide bashrc or profile to keep their stuff from killing the Xsession. No, I don't remember specifics, it's been several years and I don't work there anymore.

        In theory systemd does something similar.
        In practice... virtual memory auditing isn't turned on so it still goes BANG!

    • chithanh says:

      The OOM killer is another can of worms. Every time I read about someone finally going to handle OOM killing right, I am reminded of Andries Brouwer's classic on LKML:

      https://lwn.net/Articles/104179/

  15. frankreyes says:

    Some people, when confronted with a screen locking problem, think "I know, I'll create a new screen saver." Now they have two problems.

  16. jwz says:

    By the way, it is the case that the attack surface of the XScreenSaver daemon could be reduced further. There is still more code linked into it than is strictly necessary:

    • Some of PAM conversation could be happening in a subprocess;
    • So could the selection of, and management of, the display-mode subprocesses;
    • So could dealing with DPMS, and a few other tiny things like that.

    But those pieces are pretty tractable, and haven't been where problems tended to manifest in the past. The pieces that can't easily be removed from the daemon are anything that must happen while the mouse and keyboard are grabbed:

    • Drawing the unlock dialog, and having it interact with PAM;
    • Reading and processing events, including from server extensions;
    • Many of the timers;
    • Creating and managing windows to cover the desktop;
    • Gamma fading;
    • Dealing with hot-swapped monitors.

    And that second list, that's still a lot.

    • Robert Buj says:

      Anyway, by default the XScreenSaver daemon is explicitly banished for all desktop environments on Fedora by the package maintainer:

      https://bugzilla.redhat.com/show_bug.cgi?id=1918139
      https://bugzilla.redhat.com/show_bug.cgi?id=1918317

      • jwz says:

        Actually it looks to me like what it's doing is, "if screensaver-A is installed, do not launch screensaver-B", and that is totally reasonable because launching two and letting them fight it out would be bad. If you choose to run XScreenSaver, uninstall mate-screensaver, problem solved.

        • Robert Buj says:

          The line `OnlyShowIn=X-NODEFAULT;` filters out `xscreensaver-autostart.desktop` from startup applications (/etc/xdg/autostart), so it doesn't autostart the xscreensaver daemon on any desktop environment, since there is no desktop environment named `X-NODEFAULT`, even if neither mate-screensaver nor gnome-screesaver are installed on Fedora:

          $ cat /etc/fedora-release
          Fedora release 33 (Thirty Three)
          $ cat /etc/xdg/autostart/xscreensaver-autostart.desktop
          [Desktop Entry]
          Type=Application
          Name=xscreensaver-autostart
          Comment=Autostart xscreensaver
          Exec=/usr/libexec/xscreensaver-autostart
          Terminal=false
          X-Desktop-File-Install-Version=0.26
          OnlyShowIn=X-NODEFAULT;
          $ xscreensaver-command -lock
          xscreensaver-command: no screensaver is running on display :0

          "The OnlyShownIn entry may contain a list of strings identifying the desktop environments that MUST autostart this application, all other desktop environments MUST NOT autostart this application."

          Section "OnlyShowIn and NotShowIn Keys" of "Autostart Of Applications During Startup"
          https://specifications.freedesktop.org/autostart-spec/0.5/ar01s02.html

    • LionsPhil says:

      If you're curious, xscreenlock did indeed manage to kick out the PAM bits to a helper process: https://github.com/google/xsecurelock#security-design

      xsecurelock/helpers/authproto_pam.c is the only source that seems to bring in PAM headers.

      At least as far as wheel reinventions go, I don't think this one is square.

  17. Derpatron9000 says:

    The comments from that bug specifically directed to you are, umm, delightful.

  18. RoestVrijStaal says:

    About that stunning license-removal by GNOME devs (which they should not do at all), you did not made it easy for yourself.

    The license headers in your source files (like screenhack.h, which you suggest as default in README.hacks) read more as a "MIT"-like license, instead of bsd-2-clause, bsd-3-clause or similar BSD licenses findable at SPDX License Listing

    It might be helpful to use a SPDX-License-Identifier in your source files to communicate which license the source is licensed.

    Also to create a LICENSE.md file via one of the BSD-license-templates and alter it with your name et al.

    It sounds like overhead effort, but doing it limits licensing miscommunication.

    • RoestVrijStaal says:

      Urrrgh great, somehow the links are broken in the previous post:

      About that stunning license-removal by GNOME devs (which they should not do at all), you did not made it easy for yourself.

      The license headers in your source files (like screenhack.h, which you suggest as default in README.hacks) read more as a "MIT"-like license, instead of bsd-2-clause, bsd-3-clause or similar BSD licenses findable at SPDX License Listing

      It might be helpful to use a SPDX-License-Identifier in your source files to communicate which license the source is licensed.

      Also to create a LICENSE.md file via one of the BSD-license-templates and alter it with your name et al.

      It sounds like overhead effort, but doing it limits licensing miscommunication.

      • jwz says:

        1: Their inability to determine which license is on a piece of code is very firmly in the bucket of "their problem" and not "my problem".

        2: It doesn't matter which license (and copyright) it was that they removed and replaced with GPL (and someone else's name). You still can't do that.

        3: Whether the more-restrictive GPL itself allows BSD-variant, MIT, or X11 licenses to be commingled with GPL code is, again, firmly in the bucket of "their problem" and not "my problem". I solve this problem by not allowing any GPL code in XScreenSaver.

      • Robert Buj says:

        It's very similar to the Pre 4-clause license which is no GPL compatible as you can see on wikipedia's summary box: "Prior BSD Licene".
        https://en.wikipedia.org/wiki/BSD_licenses#Previous_license

        This adds another license issue as xscreensaver files cannot be mixed with GPL code.

        There is an open issue in MATE:
        https://github.com/mate-desktop/mate-screensaver/issues/243

        MATE works correctly if only xscreensaver is installed on the system after applying this change:
        https://github.com/mate-desktop/mate-session-manager/pull/265

        The other MATE components work if only xscreensaver is installed on system. Note the user should add xscreensaver on startup applications using mate-session-properties on Fedora as I mentioned earlier.

  19. kmeaw says:

    Why not run a screen saver in a separate instance of Xorg on a separate VT with switching disabled? If an attacker finds a way to crash it, they will be left on a blank tty with no way to switch out of it.

    This approach seems to be much more feasible than writing bug-free code. The only privileged code section would be a VT switching/locking portion. It can even be someday implemented inside the kernel as a special flag (VT_LOCKSCREEN?) for a tty that after being switched into locks manual switching and allows only the process controlling that terminal to switch out.

    • jwz says:

      It's not a bad idea, and certainly worth looking into. Downsides:

      • In my experience, switching VTs is sloooow. That means it would go like this: Screen saver is animating. You wiggle the mouse. Screen freezes solid for 2+ seconds. Unlock dialog box pops up. Hit cancel. Screen freezes solid for 2+ seconds. Screen saver continues.

      • We wouldn't' be just switching VTs, we'd also be launching an entire second X server on it. Launching an X server is also not fast. Maybe that 2+ seconds has gone to 8+ seconds now. So maybe we just keep that X server around and reuse it, but:

      • Now you've got an entire second X server running just so that one app can display one dialog box. That's a whole lot of memory to spend on that task. Helloooooo OOM-killer!

      • Or, maybe that second VT isn't running X11, maybe you do graphics in some even lower level way. Ok, so now remember that one of the most common complaints is "the unlock dialog doesn't match my desktop's color scheme". You've just made that double-complicated. Also you still have no support for input methods and virtual keyboards and screen readers and whatnot, so that hasn't gotten any better either.

      • Is the VT_SETMODE ioctl root-only? I can't tell but I'll bet it is. That means more setuid nonsense and disavowal of privs. Not insurmountable, but complicated and irritating.

      • This is really just hanging out a giant "KICK ME" sign for your video driver. Do you want to place bets on whether the driver developers have ever tested running two X servers on two VTs in any meaningful way? Does your gut tell you that when one X server is deep in the weeds of rendering a complex GL scene, then another X server gets swapped in to the frame buffer, then the original X server gets swapped back in, that the video driver is going to reconfigure the GPU properly and restore all of that state, and nothing's going to crash? My gut tells me "ha ha ha ha ha no". Have you met Nvidia? Lovely drivers.

      • Not all systems that run X11 have VTs. Do the BSDs even have them? It looks like OpenBSD has them now, but do the others, and since when? And do they work the same way? Certainly the older Unixes didn't, you only got one console on Solaris and IRIX and whatnot. This is certainly less of a concern these days, but I've been maintaining this thing for three decades so I strive for maximum portability, because I've seen so many of these OS-specific hacks come and go.

      • What even happens with RANDR on a second VT? If you open a second VT, hot-swap a monitor, then switch back to the original VT, do both get notified? What if the monitor hot-swap happens during the VT switch, in the race window when the old VT has been torn down but the new VT hasn't been set up yet? My head hurts.

      • "Maybe some day the kernel (the kernel?) will add a new flag to support my application" is not something for which you should ever be holding your breath. It's just not gonna happen.

  20. ChrisJ says:

    Maybe the simplest change in the X server would be to allow the X client to set a flag "kill the X session if I go away before I clear the flag"?

    Maybe the same behaviour could be emulated by having the screensaver fork after opening the X connection, and have the child process do the usual functionality and the parent process wait on the child and if the child exits {uncleanly,before telling all is OK}, kills the X session (via a setuid binary that verifies it's the right user and can kill X, or a daemon running as root listening on a unix domain socket that can do the same, or, I'm not sure, maybe killing xinit (running as the user, not root) would work).

    • jwz says:

      Any modification to the X server is probably impractical, given both the political quagmire and the timeframes involved.

      A meta-daemon might not be a bad idea. I wonder if there's a way to configure systemd to do this: "if job XScreenSaver exits abnormally, restart job desktop session."

      • ChrisJ says:

        The reason why I said that the screensaver should fork after opening the X connection is so that if the child crashes, the screensaver window will still be held in place (as there was no command sent to close it, and the file handle is still open; at least that's how I understand X behaves). If you have systemd check on xscreensaver and then kill the session there may be a race during which the lock window may be gone. But maybe I don't know enough details.

  21. balsoft says:

    > If you are not running XScreenSaver on Linux, then it is safe to assume that your screen does not lock.

    What is the situation with tools like slock/vlock? Are they insecure too? I haven't read their source code, but it seems to me like they follow most of the principles you've highlighted in your article.

    • Commandhat says:

      slock has render code almost entirely stolen from debian-screensaver, so I do not have high hopes for their screen locking implementation being decent.

  22. genewitch says:

    the mate-desktop contributor raveit65 is in both of the issues threads mentioned in the past 4 weeks on this site, saying stuff like "we should ignore this report" about this licensing issue.

    I license CC0 whenever i push some silly script or whatever, because i don't understand licenses other than CC, which, i think avoids all of these issues. However, i also don't fork stuff, edit it, and then change license to CC0 or remove the original author's name. That's what thieves do (see all of the SVG "repo" sites, and pyfiglet https://github.com/pwaller/pyfiglet/issues/58 ).

    All i can do as a nobody is put a thumbs-down emojii on their comments. Their cavalier attitude is pretty offensive, even from the very outside peering in.

    P.S. I found your site via Dan Luu's site, from his third post ever, and i appreciate the content and the comments very much. Thank you!

  23. no name says:

    Xfce Screensaver is a fork of MATE Screensaver so maybe it has the same problems.

  24. Dale Magee says:

    Wow. At first when I saw this I literally thought I was looking at a decade old post. Then I noticed the "2021 edition" and facepalmed.

    It boggles my mind that this is still a thing after all these years. I'm just glad I found your writing and have made a point of installing xscreensaver ever since. Some of us do listen.

    But learning that your code was stolen really infuriates me on your behalf. The lack of ethics on display and the attitude of "oh it's just a little bit of copyright infringement" is really astounding to me.

    I hope they do make it right for you. But I'm not holding my breath.

    Thanks so much for all your great work on xscreensaver over the years Jamie, it's been indispensable to me for ~15 years now.

  • Previously