Useless setrlimit(RLIMIT_CORE, ...) and typo in mainconfig.c
Hi, when coredumps are enabled in the config file, the code in mainconfig.c does an setrlimit(RLIMIT_CORE, ...) using the core limits retrieved by an earlier getrlimit(RLIMIT_CORE, ...). This looks useless to me. I think the original intention of the setrlimit() was to actually enable core dumping in case the server is started with core dump limit set to 0 but coredumps enabled in the config file. Further, a prctl() call to set the dumpable flag of the process is protected by HAVE_SYS_PRTCL_H which should be HAVE_SYS_PRCTL_H. Even with the typo fixed, the code will never be run, because configure does not check for prctl.h yet. I have attached a suggested patch (not including configure / prctl.h). Enrik Index: src/main/mainconfig.c =================================================================== RCS file: /source/radiusd/src/main/mainconfig.c,v retrieving revision 1.127 diff -u -r1.127 mainconfig.c --- src/main/mainconfig.c 9 Oct 2007 14:45:59 -0000 1.127 +++ src/main/mainconfig.c 15 Oct 2007 14:41:07 -0000 @@ -42,7 +42,7 @@ #include <grp.h> #endif -#ifdef HAVE_SYS_PRTCL_H +#ifdef HAVE_SYS_PRCTL_H #include <sys/prctl.h> #endif @@ -577,7 +577,7 @@ #endif if (allow_core_dumps) { -#ifdef HAVE_SYS_PRTCL_H +#ifdef HAVE_SYS_PRCTL_H #ifdef PR_SET_DUMPABLE if (prctl(PR_SET_DUMPABLE, 1) < 0) { radlog(L_ERR,"Cannot enable core dumps: prctl(PR_SET_DUMPABLE) failed: '%s'", @@ -587,6 +587,7 @@ #endif #ifdef HAVE_SYS_RESOURCE_H + core_limits.rlim_cur = core_limits.rlim_max; if (setrlimit(RLIMIT_CORE, &core_limits) < 0) { radlog(L_ERR, "Cannot update core dump limit: %s", strerror(errno));
Enrik Berkhan wrote:
when coredumps are enabled in the config file, the code in mainconfig.c does an setrlimit(RLIMIT_CORE, ...) using the core limits retrieved by an earlier getrlimit(RLIMIT_CORE, ...). This looks useless to me.
It's there for paranoia. Maybe the limits get reset after setuid().
I think the original intention of the setrlimit() was to actually enable core dumping in case the server is started with core dump limit set to 0 but coredumps enabled in the config file.
No. If the parent shell has the core limit set to zero, the server should respect that.
Further, a prctl() call to set the dumpable flag of the process is protected by HAVE_SYS_PRTCL_H which should be HAVE_SYS_PRCTL_H. Even
Fixed.
with the typo fixed, the code will never be run, because configure does not check for prctl.h yet.
I can go fix that, too.
I have attached a suggested patch (not including configure / prctl.h).
I've committed some code clarifying the code paths, and doing additional checks: - if debug_flag > 0, core dumps are always allowed (ulimit inherited from parent shell) - if NOT debugging AND core dumps are disabled, AND we didn't do setuid(), then disable core dumps. (setuid disables core dumps by itself) - if we're in daemon mode, AND core dumps are enabled, AND we did setuid(), then re-enable core dumps. - otherwise, core limit is inherited from parent process. Alan DeKok.
participants (2)
-
Alan DeKok -
Enrik Berkhan