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

mlockall() call needs to be removed #55

Open
keszybz opened this issue Jan 15, 2019 · 9 comments
Open

mlockall() call needs to be removed #55

keszybz opened this issue Jan 15, 2019 · 9 comments

Comments

@keszybz
Copy link

keszybz commented Jan 15, 2019

mlockall() is generally a bad idea and certainly has no place in a graphical program. A program like this uses lots of memory and it is crucial that this memory can be paged out to relieve memory pressure.
It also doesn't have the desired security effect. See https://bugzilla.redhat.com/show_bug.cgi?id=1662857#c18 for a longer explanation.

If you want to keep memory locked for the vestigial security effect, apply mlock() carefully for the space used to store the password, and/or use explicit_bzero to erase afterwards.

mlockall() also prevents the machine from starting if the memory lock limit is raised high enough, see the linked bug.

@robert-ancell
Copy link
Collaborator

The mlockall is not used on the graphical component or the daemon - it's used on the session management process that then forks the actual session.

I don't actually know how the mlockall call works - it was added many years ago by @mikix. I don't know if it's still an appropriate way of securing things - the password is likely to be in loads of other memory locations (e.g. the greeter process).

I'm not opposed to removing it - I just need some confirmation that's the right thing to do.

@leigh123linux
Copy link

@robert-ancell mlockall breaks lightdm with systemd-240.

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

@mikix
Copy link
Contributor

mikix commented Jan 16, 2019

@robert-ancell I do not remember the context for that change, sorry :(

@khurshid-alam
Copy link

khurshid-alam commented Jan 18, 2019

It has something to do with password is stored in the memory during hibernation, I tried to find the bug, but couldn't find any.

In any case if mlockall() is really the problem, then why does LimitMEMLOCK works with systemd-240 ? And why systemd made that specific change ?

@keszybz
Copy link
Author

keszybz commented Jan 18, 2019

mlockall(2) says:

If MCL_FUTURE has been specified, then a later system call (e.g., mmap(2),
sbrk(2), malloc(3)), may fail if it would cause the number of locked bytes to
exceed the permitted maximum (see below). In the same circumstances, stack
growth may likewise fail: the kernel will deny stack expansion and deliver a
SIGSEGV signal to the process.

As described in https://bugzilla.redhat.com/show_bug.cgi?id=1662857#c9:

With systemd version 239 the ulimit for RLIMIT_MEMLOCK was set to 16 MiB and therefore the mlockall call would fail. This is lucky becasue the subsequent mmap would not fail.

With systemd version 240 the RLIMIT_MEMLOCK is now set to 64 MiB and now the mlockall no longer fails. However, it not possible to mmap in all the memory and because that would still exceed the MEMLOCK limit.

So it was essentially working by luck, and this luck has run out.

As to the hibernation part:
mlockall(2) says:

Memory locking has two main applications: real-time algorithms and high-security
data processing. [...]
(But be aware that the suspend mode
on laptops and some desktop computers will save a copy of the system's RAM to
disk, regardless of memory locks.)

(It talks about suspend, but we nowadays call this hibernation.) Anyway, this makes mlockall pointless, as described in https://bugzilla.redhat.com/show_bug.cgi?id=1662857#c18.

@khurshid-alam
Copy link

khurshid-alam commented Feb 8, 2019

@robert-ancell
Atm, With systemd-240, unity-greeter can not start goes into infinite loop. Can you set LimitMEMLOCK and make a release?

Update: It seems even with LimitMEMLOCK=16777216, unity-greeter can not start. I am just adding it lightdm.service fil. Is there any limit-lock gsettings key for lightdm ?

[Unit]
Description=Light Display Manager
Documentation=man:lightdm(1)
Conflicts=getty@tty7.service plymouth-quit.service
After=systemd-user-sessions.service getty@tty7.service plymouth-quit.service

[Service]
# temporary safety check until all DMs are converted to correct
# display-manager.service symlink handling
ExecStartPre=/bin/sh -c '[ "$(basename $(cat /etc/X11/default-display-manager 2>/dev/null))" = "lightdm" ]'
ExecStart=/usr/sbin/lightdm
Restart=always
BusName=org.freedesktop.DisplayManager
LimitMEMLOCK=16777216

@khurshid-alam
Copy link

khurshid-alam commented Feb 9, 2019

I added this /etc/lightdm.conf

[LightDM]
lock-memory=false

And then removed mlockall from unity-greeter and now greeter starts. But now logout fails. :(

update: Logout issue is now fixed

@LionNatsu
Copy link

I also found this today. I'm not sure this issue is part of lightdm because I found mlockall in lightdm-gtk-greeter. lightdm-gtk-greeter aborted and core dumped because of mmap=EAGAIN.
But just a FYI, The mlockall() was introduced since https://git.launchpad.net/lightdm-gtk-greeter/commit/?id=4c8782a9d66cd885c74b6f30ad33e053a9079d65 (in 2014)

The most different thing is, lightdm runs as root ignoring the RLIMIT_MEMLOCK (64MB here), but the greeter runs as user "lightdm" and crashes now. I do think replacing mlockall with fine-grained mlock is the best way for the code base, but it seems still need a long time for them to fix while many users cannot login now.

@khurshid-alam, adding LimitMEMLOCK=infinity is not an appropriate option because rlimit is inheritable. It affects all processes spawn from lightdm that is all user processes will have no limitation for mlock by default. The worst situation could be a unprivileged process locked down, say 90% memory, and this end up as DoS.

@khurshid-alam
Copy link

khurshid-alam commented Mar 1, 2019

@LionNatsu

That call has been removed from both unity-greeter and sleek-greeter. gtk-greeter should also remove that.

sunweaver added a commit to ArcticaProject/arctica-greeter that referenced this issue Mar 17, 2019
  Protect memory from being paged to disk, as we deal with passwords

  According to systemd-dev,

  "mlockall() is generally a bad idea and certainly has no place in a graphical program.
  A program like this uses lots of memory and it is crucial that this memory can be paged
  out to relieve memory pressure."

  With systemd version 239 the ulimit for RLIMIT_MEMLOCK was set to 16 MiB
  and therefore the mlockall call would fail. This is lucky becasue the subsequent mmap would not fail.

  With systemd version 240 the RLIMIT_MEMLOCK is now set to 64 MiB
  and now the mlockall no longer fails. However, it not possible to mmap in all
  the memory and because that would still exceed the MEMLOCK limit.
  "
  See https://bugzilla.redhat.com/show_bug.cgi?id=1662857 &
  canonical/lightdm#55

  RLIMIT_MEMLOCK = 64 MiB means, arctica-greeter will most likely fail with 64 bit and
  will always fail on 32 bit systems.

  Hence we better disable it.

  Ported from Unity Greeter / Slick Greeter.

  https://bugs.launchpad.net/ubuntu/+source/unity-greeter/+bug/1815493
  linuxmint/slick-greeter#127
yetist pushed a commit to srcmirror/lightdm-gtk-greeter that referenced this issue Oct 21, 2019
Protect memory from being paged to disk, as we deal with passwords

According to systemd-dev,

"mlockall() is generally a bad idea and certainly has no place in a graphical program.
A program like this uses lots of memory and it is crucial that this memory can be paged
out to relieve memory pressure."

With systemd version 239 the ulimit for RLIMIT_MEMLOCK was set to 16 MiB
and therefore the mlockall call would fail. This is lucky becasue the subsequent mmap would not fail.

With systemd version 240 the RLIMIT_MEMLOCK is now set to 64 MiB
and now the mlockall no longer fails. However, it not possible to mmap in all
the memory and because that would still exceed the MEMLOCK limit.
"
See https://bugzilla.redhat.com/show_bug.cgi?id=1662857 &
canonical/lightdm#55

RLIMIT_MEMLOCK = 64 MiB means, arctica-greeter will most likely fail with 64 bit and
will always fail on 32 bit systems.

Hence we better disable it.

Ported from Unity Greeter / Slick Greeter / Artica Greeter

https://bugs.launchpad.net/ubuntu/+source/unity-greeter/+bug/1815493
linuxmint/slick-greeter#127
davidmhewitt added a commit to elementary/greeter that referenced this issue May 5, 2020
This seemingly causes the greeter to fail to start on architectures other than amd64 and has been removed from all of the other greeters due to it causing sporadic issues with various versions of systemd.

Apparently it doesn't really offer much extra security (which I think was the original point of using it). Discussion here:
canonical/lightdm#55 (comment)
tintou pushed a commit to elementary/greeter that referenced this issue May 8, 2020
This seemingly causes the greeter to fail to start on architectures other than amd64 and has been removed from all of the other greeters due to it causing sporadic issues with various versions of systemd.

Apparently it doesn't really offer much extra security (which I think was the original point of using it). Discussion here:
canonical/lightdm#55 (comment)
danirabbit pushed a commit to elementary/greeter that referenced this issue Mar 8, 2021
This seemingly causes the greeter to fail to start on architectures other than amd64 and has been removed from all of the other greeters due to it causing sporadic issues with various versions of systemd.

Apparently it doesn't really offer much extra security (which I think was the original point of using it). Discussion here:
canonical/lightdm#55 (comment)
rubenquidam added a commit to trisquelgnulinux/package-helpers that referenced this issue Feb 22, 2022
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

No branches or pull requests

6 participants