Skip to content

Conversation

@pmenzel
Copy link
Contributor

@pmenzel pmenzel commented Aug 17, 2026

Found by claude-opus-5 while working on something else.

Resolves: #183

exec_reaper() means to switch core dumps off for a job, but assigns
rlim_cur twice instead of assigning rlim_max once:

    rlim.rlim_cur = 0;
    rlim.rlim_cur = 0;
    if (setrlimit(RLIMIT_CORE,  &rlim) == -1)

rlim_max is therefore still the value left over from the RLIMIT_DATA
call three lines above, so the hard limit becomes the job's booked
memory instead of zero. A job can raise its soft limit again and dump a
core of that size into the working directory, which is the opposite of
what the code intends. Replicating the sequence for a job booked with
4096 MiB:

    inherited RLIMIT_CORE soft=0 hard=42949672960
    current:  RLIMIT_CORE soft=0 hard=4294967296
    fixed:    RLIMIT_CORE soft=0 hard=0

Assign rlim_max, so that the limit is irrevocable for the job.

Fixes: 511439f ("mxqd: Enforce limits using setrlimit()")
Assisted-by: Claude Opus 5 (claude-opus-5)
@donald
Copy link
Contributor

donald commented Aug 18, 2026

This surly fixes the code to what was intended, but is it what we want? I can see that we want to prevent users from producing core files unintentionally, but core dumps have valid usages and we shouldn't forbid it if it is asked for explicitly. So maybe set the soft limit only and leave the hard limit as reported by getrlimit() ?

btw: Currently we seem to have disabled core dumps globally (/proc/sys/kernel/core_pattern = |/bin/false). So in our current environment the user still couldn't producer coredumps even if we leave the soft limit. Maybe we should remove that feature from mxq completely?

  • btw.btw 1: Where do we set core_pattern?
  • btw.btw 2: Wouldn't an empty string also disabled core dumps, do we really need a /bin/false pipe?

@pmenzel
Copy link
Contributor Author

pmenzel commented Aug 18, 2026

This surly fixes the code to what was intended, but is it what we want? I can see that we want to prevent users from producing core files unintentionally, but core dumps have valid usages and we shouldn't forbid it if it is asked for explicitly. So maybe set the soft limit only and leave the hard limit as reported by getrlimit()?

Before commit 88d9aa4486863e888d489eb72cd4c21154abd588 there was a comment:

/* disable core files */

88d9aa4#diff-6055fef16441d98121b23582ef858437a0c22e1c31af49d47008601cece25f20L1055

btw: Currently we seem to have disabled core dumps globally (/proc/sys/kernel/core_pattern = |/bin/false). So in our current environment the user still couldn't producer coredumps even if we leave the soft limit. Maybe we should remove that feature from mxq completely?

* btw.btw 1: Where do we set core_pattern?

Good question. I wasn’t able to find it. I always thought under /etc/sysctl.conf or /etc/sysctl.d/50-coredump.conf, but that’s not the case. Need to dig further.

* btw.btw 2: Wouldn't an empty string also disabled core dumps, do we really need a /bin/false pipe?

Probably not.

@pmenzel
Copy link
Contributor Author

pmenzel commented Aug 18, 2026

btw: Currently we seem to have disabled core dumps globally (/proc/sys/kernel/core_pattern = |/bin/false). So in our current environment the user still couldn't producer coredumps even if we leave the soft limit. Maybe we should remove that feature from mxq completely?

* btw.btw 1: Where do we set core_pattern?

Good question. I wasn’t able to find it. I always thought under /etc/sysctl.conf or /etc/sysctl.d/50-coredump.conf, but that’s not the case. Need to dig further.

* btw.btw 2: Wouldn't an empty string also disabled core dumps, do we really need a /bin/false pipe?

Probably not.

We have

$ ls -lh /etc/sysctl.d/50-coredump.conf
lrwxrwxrwx 1 root root 9 Apr 23  2018 /etc/sysctl.d/50-coredump.conf -> /dev/null

and systemd does the rest:

https://github.com/systemd/systemd/blob/f1d0952a125b96b7ab2f1ff29a87448ade8ac29b/src/shared/coredump-util.c#L181-L191

@donald
Copy link
Contributor

donald commented Aug 18, 2026

https://github.com/systemd/systemd/blob/f1d0952a125b96b7ab2f1ff29a87448ade8ac29b/src/shared/coredump-util.c#L181-L191

Oh dear, that code is a bit surprising. Next question would be why our systemd package masks the file. It's not explicit in the bee file. Maybe ENABLE_COREDUMP off by default and triggers that, but I didn't find it.

@pmenzel
Copy link
Contributor Author

pmenzel commented Aug 18, 2026

https://github.com/systemd/systemd/blob/f1d0952a125b96b7ab2f1ff29a87448ade8ac29b/src/shared/coredump-util.c#L181-L191

Oh dear, that code is a bit surprising. Next question would be why our systemd package masks the file. It's not explicit in the bee file. Maybe ENABLE_COREDUMP off by default and triggers that, but I didn't find it.

I think to remember to have masked it in 2018 directly on the distmaster. You are right, that it should be moved to mxtools.

@donald
Copy link
Contributor

donald commented Aug 18, 2026

"why our systemd package masks the file" : I confused the wild file /etc/sysctl.d/50-coredump.conf with /etc/systemd/coredump.conf from the systemd package.

@donald
Copy link
Contributor

donald commented Aug 19, 2026

So maybe set the soft limit only and leave the hard limit as reported by getrlimit() ?

For yet unknown reason, when a command ist started with sudo, it has core hard and soft limit 0. So when mxqd is started with "sudo", its soft limit is also 0. So it currently doesn't make a difference whether we set the soft limit to or leave it as is (undefined, maybe 0).

buczek@claptrap:~/git/mxq (next)$ grep core /proc/self/limits
Max core file size        unlimited            unlimited            bytes     
buczek@claptrap:~/git/mxq (next)$ sudo grep core /proc/self/limits
Max core file size        0                    0                    bytes     
buczek@claptrap:~/git/mxq (next)$ 

@donald
Copy link
Contributor

donald commented Aug 30, 2026

For yet unknown reason, when a command ist started with sudo, it has core hard and soft limit 0.

sudoers(5):

If there is no system mechanism to set per‐user resource
limits, the command will run with the same limits as the invoking user. The one
exception to this is the core dump file size, which is set by sudoers to 0 by
default. Disabling core dumps by default makes it possible to avoid potential
security problems where the core file is treated as trusted input.

We currently don't have pam_limits.so in /etc/pam.d/sudo.

Sign in to join this conversation on GitHub.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

exec_reaper(): rlim_cur assigned twice, the RLIMIT_CORE hard limit is never zeroed

2 participants