openssh bugs - May 2008 - [Bug 1470] New: adjust Linux out-of-memory killer to stop sshd being killed

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

           Summary: adjust Linux out-of-memory killer to stop sshd being
                    killed
    Classification: Unclassified
           Product: Portable OpenSSH
           Version: 5.0p1
          Platform: All
               URL: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=34176
                    7
        OS/Version: Linux
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: sshd
        AssignedTo: bitbucket at mindrot.org
        ReportedBy: cjwatson at debian.org


Created an attachment (id=1507)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1507)
adjust Linux out-of-memory killer

In some out-of-memory situations, the Linux kernel will look for a
process to kill, employing some heuristics to try to guess what will
help. It doesn't always get this right and can occasionally end up
killing innocent bystanders (though as noted in the referenced bug log
it's possible to tweak this to be more accurate).

It is useful to instruct the OOM killer never to kill sshd, since
almost everyone wants it to keep on running so that they have a chance
of dealing with the problem remotely. Originally I implemented this in
an init script, by getting sshd's pid and writing to
/proc/$pid/oom_adj, but Vaclav Ovsik pointed out in
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=480020 that that ends
up immortalising child processes too.

The attached patch is based on Vaclav's, though I tidied it up and
moved chunks of it into openbsd-compat/port-linux. The use of an
environment variable for configuration is a bit odd. I didn't feel good
about introducing a port-specific configuration file key, and the
values you write into oom_adj have a pretty bizarre syntax (documented
in
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/filesystems/proc.txt;hb=HEAD)
which I think is unlikely to be portable to other systems. I'd
appreciate any better ideas here.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470


Colin Watson <cjwatson at debian.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|5.0p1                       |-current




-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470


Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org




--- Comment #1 from Damien Miller <djm at mindrot.org>  2009-01-22
10:38:15 ---
some feedback on the diff:

Isn't /proc/self/oom_adj just an integer? I don't see any bizarre
syntax in the referenced document, but the available docs are seriously
deficient... If it is just an integer then why not just read and write
it using stdio?

I don't think that it should be controlled by an environment variable -
sshd should just set the "never kill me" flag for the master process
unconditionally.

Failure to open() /proc/self/oom_adj shouldn't throw an logit() - this
will just spam older Linux systems that lack the control or don't have
/proc mounted.

The port-linux.c code is incorrect: it doesn't handle EINTR errors on
read()/write() - it should use atomicio() if it can't use stdio.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

Scott Emery <ad6ry at yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ad6ry at yahoo.com

--- Comment #2 from Scott Emery <ad6ry at yahoo.com> 2009-10-10 10:25:48
EST ---
Why is the oom_adj value being passed in as an environment variable?
I would have expected it to be a flag in /etc/ssh/sshd_config. Is it
bad form to have OS specific sshd_config flags?

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

--- Comment #3 from Iain Morgan <imorgan at nas.nasa.gov> 2009-10-28
11:28:14 EST ---
Created an attachment (id=1712)
 --> (https://bugzilla.mindrot.org/attachment.cgi?id=1712)
Revised patch for Linux OOM killer

Updated the previous patch based on Damien's feedback in Comment #1.
Limited testidng indicates that the patch works. The one oddity is that
the message logged by verbose() when restoring the original oom_adj
value shows up three times in /var/log/syslog.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at zip.com.au
             Blocks|                            |1626

--- Comment #4 from Darren Tucker <dtucker at zip.com.au> 2009-12-07
12:02:36 EST ---
Add to list for 5.4

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

--- Comment #5 from Darren Tucker <dtucker at zip.com.au> 2009-12-07
12:04:46 EST ---
Created an attachment (id=1740)
 --> (https://bugzilla.mindrot.org/attachment.cgi?id=1740)
openssh-linux-oom_kill.patch

Use the platform_* hooks to avoid sprinkling more #ifdefs into the main
code.
Move the saved value to port-linux.c.  Add LINUX_ to the define since
it is
Linux-specific.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1740|                            |ok?(djm at mindrot.org)
               Flag|                            |

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

--- Comment #6 from Darren Tucker <dtucker at zip.com.au> 2009-12-07
12:37:17 EST ---
Created an attachment (id=1741)
 --> (https://bugzilla.mindrot.org/attachment.cgi?id=1741)
openssh-linux-oom_kill.patch

Create a platform_pre_listen hook and use that for the oom adjust.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1741|0                           |1
           is patch|                            |
   Attachment #1741|                            |ok?(djm at mindrot.org)
               Flag|                            |

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1507|0                           |1
        is obsolete|                            |

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1740|0                           |1
        is obsolete|                            |
   Attachment #1740|ok?(djm at mindrot.org)        |
               Flag|                            |

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

--- Comment #7 from Darren Tucker <dtucker at zip.com.au> 2009-12-07
17:07:37 EST ---
Created an attachment (id=1742)
 --> (https://bugzilla.mindrot.org/attachment.cgi?id=1742)
openssh-linux-oom_kill.patch

Don't try to restore a value that we did not save.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

--- Comment #8 from Darren Tucker <dtucker at zip.com.au> 2009-12-07
17:14:16 EST ---
(In reply to comment #3)
> The one oddity is that
> the message logged by verbose() when restoring the original oom_adj
> value shows up three times in /var/log/syslog.
I think I can explain that:

#1: Despite what's implied by the message oom_adjust_setup() actually
logs the saved value not the set value:

+    verbose("Set %s to %d", 
+       OOM_ADJ_PATH, oom_adj_save);

#2: oom_adjust_setup() gets called a second time when sshd re-execs
itself to randomize its address space.

#3: the real call to oom_adjust_restore()

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1742|                            |ok+
               Flag|                            |

--- Comment #9 from Damien Miller <djm at mindrot.org> 2009-12-08 11:09:20
EST ---
(From update of attachment 1742)
>Index: openbsd-compat/port-linux.c
>==================================================================>RCS
file: /var/cvs/openssh/openbsd-compat/port-linux.c,v
>retrieving revision 1.6
>diff -u -p -r1.6 port-linux.c
>--- openbsd-compat/port-linux.c	24 Oct 2009 04:04:13 -0000	1.6
>+++ openbsd-compat/port-linux.c	7 Dec 2009 06:06:11 -0000
>@@ -27,8 +27,15 @@
> #include <stdarg.h>
> #include <string.h>
> 
>-#ifdef WITH_SELINUX
>+#if defined(LINUX_OOM_ADJUST) || defined(WITH_SELINUX)
> #include "log.h"
>+#endif
>+
>+#ifdef LINUX_OOM_ADJUST
>+#include <stdio.h>
>+#endif
>+
I wouldn't bother slicing and dicing the header inclusion based on
preprocessor symbols. There is little cost to including them
unconditionally, or perhaps conditionally on the union of all supported
symbols for this file.
>+#ifdef LINUX_OOM_ADJUST
>+#define OOM_ADJ_PATH	"/proc/self/oom_adj"
>+#define OOM_ADJ_NOKILL	-17  /* magic value to disable OOM killer */
FYI, -17 is documented in Documentation/filesystems/proc.txt in the
Linux source. A stable URL for this is
http://lxr.linux.no/#linux+v2.6.32/Documentation/filesystems/proc.txt
if you want to include it.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #10 from Darren Tucker <dtucker at zip.com.au> 2009-12-08
13:41:31 EST ---
(In reply to comment #9)
> perhaps conditionally on the union of all supported
> symbols for this file.
Done.
> FYI, -17 is documented in Documentation/filesystems/proc.txt in the
> Linux source. A stable URL for this is
> http://lxr.linux.no/#linux+v2.6.32/Documentation/filesystems/proc.txt
> if you want to include it.
I know, I read it :-).  Reference added.

Thanks all, the patch has been applied and will be in 5.4p1.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

--- Comment #11 from Colin Watson <cjwatson at debian.org> 2010-02-28
07:49:34 EST ---
The patch as applied has one flaw that I can see.  Apparently some
virtualisation containers (vserver/OpenVZ) don't allow processes to
write to /proc/self/oom_adj, and will return an error code if they try.
 It would be a shame for sshd to unconditionally log an error on such
systems; I think this was probably the main benefit of having it
controlled by an environment variable, so that they could turn this
feature off.

How about just lowering errors from writing to /proc/self/oom_adj to
debug1(), rather than logit()?

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

--- Comment #12 from Darren Tucker <dtucker at zip.com.au> 2010-03-01
15:53:27 EST ---
(In reply to comment #11)
> The patch as applied has one flaw that I can see.  Apparently some
> virtualisation containers (vserver/OpenVZ) don't allow processes to
> write to /proc/self/oom_adj, and will return an error code if they try.
>  It would be a shame for sshd to unconditionally log an error on such
> systems; I think this was probably the main benefit of having it
> controlled by an environment variable, so that they could turn this
> feature off.
> 
> How about just lowering errors from writing to /proc/self/oom_adj to
> debug1(), rather than logit()?
I've lowered them to verbose(), same as the other calls.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

--- Comment #13 from Colin Watson <cjwatson at debian.org> 2010-03-01
20:56:31 EST ---
Thanks, that should do the job.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1470

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #14 from Darren Tucker <dtucker at zip.com.au> 2010-03-26
10:51:49 EST ---
With the release of 5.4p1, this bug is now considered closed.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.