Page MenuHomePhabricator

$wgMaxShellMemory won't set ulimit on non-Linux POSIX Operating Systems
Open, MediumPublicFeature

Description

Author: rabe

Description:
make ulimit4.sh /bin/sh aware

$wgMaxShellMemory isn't respected on POSIX aware Operating Systems other than Linux.

In includes/GlobalFunctions.php around line 2785 we have <code>if ( php_uname( 's' ) == 'Linux' ) {</code> around the handling of using "ulimit4.sh"

Calling "convert" without any limits set will kill your server.
also see: http://www.mediawiki.org/wiki/Manual_talk:$wgMaxShellMemory#Caution:_This_is_LINUX_only.21_Won.27t_work_with_POSIX_like_systems_e.g._FreeBSD

I patched this on my MW-1.20.3 and it seems fine:

Please review this patches.

I set severity to "major" as this will kill wikis running on FreeBSD by just uploading one complex/broken SVG.

Regards
Raphael


Version: 1.20.x
Severity: enhancement
OS: FreeBSD

Attached:

Details

Reference
bz47781

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 1:16 AM
bzimport set Reference to bz47781.
bzimport added a subscriber: Unknown Object (MLST).

rabe wrote:

Make wfShellExec() FreeBSD aware.

Attached:

Changing component to general. Not specificly a file management issue

ulimit4.sh is no longer in current master code. You may want to test our current code and see whether it works in your environment.

rabe wrote:

(In reply to comment #3)

ulimit4.sh is no longer in current master code. You may want to test our
current code and see whether it works in your environment.

However, wfShellExec() should not only work with Linux and /bin/bash when dealing with process limits.

Please provide some pointer how this will be done in future releases.

Regards
Raphael

Again this is not about File management, which refers to files uploaded by users.

A similar script is now at includes/limit.sh . See https://gerrit.wikimedia.org/r/gitweb?p=mediawiki/core.git;a=blob;f=includes/limit.sh;hb=refs/heads/master .

"ulimit -v" is not ideal for memory limiting, and was only used due to limitations of the Linux kernel. We've now implemented support for cgroups, which is a far superior option for Linux.

Maybe for FreeBSD, -m (RSS) should be used, with -w (swap size) set to zero.

rabe wrote:

(In reply to comment #6)

"ulimit -v" is not ideal for memory limiting, and was only used due to
limitations of the Linux kernel. We've now implemented support for cgroups,
which is a far superior option for Linux.

Maybe for FreeBSD, -m (RSS) should be used, with -w (swap size) set to zero.

  • Everything using hardcoded "/bin/bash" will break with non-Linux OS'es.
  • Everything using hardcoded non-standard path/tool will break on most standard OS'es.

(In reply to comment #7)

  • Everything using hardcoded "/bin/bash" will break with non-Linux OS'es.
  • Everything using hardcoded non-standard path/tool will break on most

standard
OS'es.

Update them so they aren't hardcoded?

rabe wrote:

There SHOULD be an alternative method to cgroups by using POSIX ulimit. Better have ulimits set instead of nothing.

Maybe the method is decided in one global limit.sh or in PHP depending on OS or $wgWhateverLimitMethod="ulimit" configuration

Using timeout is cool stuff, but the executable in FreeBSD is located at /usr/local/bin/gtimeout (provided by coreutils-8.20_2 in my case). The script SHOULD check for the existence of this binary. If there's really the need of a (modern) bash, then make /bin/bash variable, e.g. /usr/local/bin/bash

My bugreport above matches 1.20.3 and will match in another way when using the new cool cgroups stuff AFAICS here.

Regards
Raphael

(In reply to comment #7)

  • Everything using hardcoded "/bin/bash" will break with non-Linux OS'es.
  • Everything using hardcoded non-standard path/tool will break on most

standard
OS'es.

I'm aware of that. That's why wfShellExec() doesn't attempt to use the script on any operating system other than Linux.

I think FreeBSD support would be a new feature; its absence is not a bug. I'm trying to work out how that feature should best be implemented. I thought you might have some useful information on that, since you are evidently a FreeBSD user.

Note that support for memory limiting on Mac OS X and Windows is also absent.

rabe wrote:

Hi Tim,

my 2 patches (against ulimit4.sh and wfShellExec()) should make this code Linux-*and*-BSD aware (I guess it should work this way on any POSIX compatible system with /bin/sh).

The main difference is, that /bin/sh ulimit cannot set multiple resource limits at once. Setting 3 limits require 3x ulimit. That's all.

If you do 3x ulimit instead of one long ulimit command, this should work with (any?) POSIX-/bin/sh so you might replace /bin/bash with /bin/sh generally.

FreeBSD doesn't implement cgroups, so using "good old ulimit" is better than nothing here. Your new limit.sh might "detect" this inability of cgroups and may fall back to "traditional ulimits" (like ulimit4.sh in my patch). This would be great.

I cannot send you patches for your new code yet because I have no "unstable" mediawiki installed anywhere, I just cannot fix such things.

Regards
Raphael

Raphael: Any chance to put these patches into Gerrit?

Raphael: Could you put these patches into Gerrit?
See https://www.mediawiki.org/wiki/Developer_access and
to submit this as a Git branch directly into Gerrit:

https://www.mediawiki.org/wiki/Git/Tutorial

Putting your branch in Git makes it easier to review it quickly.

Aklapper changed the subtype of this task from "Task" to "Feature Request".Feb 4 2022, 12:24 PM