From c2ac69b2d5e186d5215ea822d8cbcfacf07d22e8 Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Sun, 12 Dec 2021 22:51:59 -0800 Subject: [PATCH] Use dynamic maximum configuration values when necessary (#370) This commit fixes an issue with how ksh was obtaining the value of NGROUPS_MAX. On some systems this setting can be changed (e.g., on illumos adding 'set ngroups_max=32' to /etc/system then rebooting changes NGROUPS_MAX from 16 to 32). Ksh was using NGROUPS_MAX with the assumption it's a static value, which could cause issues on systems where it isn't static. This bugfix is inspired by the one from , although it has been expanded a bit to account for OPEN_MAX as well. src/cmd/ksh93/sh/init.c, src/lib/libcmd/fds.c: - Rename the getconf() macro to astconf_long() and move it to ast.h to prevent redundancy. Other sections of the code have been modified to use this macro for astconf() to account for dynamic settings. - An equivalent macro for unsigned long values (astconf_ulong) has been added. - Prefer sysconf(3) where available. It has better performance as it returns a numeric value directly instead of via string conversion. - The astconf_long and astconf_ulong macros have been documented in the ast(3) man page. --- src/cmd/ksh93/bltins/test.c | 4 ++-- src/cmd/ksh93/features/externs | 2 +- src/cmd/ksh93/sh/init.c | 9 +++------ src/cmd/ksh93/sh/io.c | 2 +- src/cmd/ksh93/sh/suid_exec.c | 2 +- src/lib/libast/comp/eaccess.c | 2 +- src/lib/libast/include/ast.h | 21 +++++++++++++++++++++ src/lib/libast/man/ast.3 | 28 +++++++++++++++++++++++++++- src/lib/libast/misc/cmdarg.c | 2 +- src/lib/libast/path/pathtemp.c | 2 +- src/lib/libast/stdio/vfwscanf.c | 2 +- src/lib/libast/string/fmtclock.c | 6 +++--- src/lib/libast/vmalloc/malloc.c | 13 ++++++++++--- src/lib/libcmd/fds.c | 11 +++++++---- src/lib/libcmd/id.c | 2 +- 15 files changed, 81 insertions(+), 27 deletions(-) diff --git a/src/cmd/ksh93/bltins/test.c b/src/cmd/ksh93/bltins/test.c index ffd07969b..e2e6dc641 100644 --- a/src/cmd/ksh93/bltins/test.c +++ b/src/cmd/ksh93/bltins/test.c @@ -666,7 +666,7 @@ skip: if((maxgroups=getgroups(0,(gid_t*)0)) <= 0) { /* pre-POSIX system */ - maxgroups=NGROUPS_MAX; + maxgroups = (int)astconf_long(CONF_NGROUPS_MAX); } } groups = (gid_t*)stakalloc((maxgroups+1)*sizeof(gid_t)); @@ -680,7 +680,7 @@ skip: } } } -# endif /* _lib_getgroups */ +#endif /* _lib_getgroups */ if(statb.st_mode & mode) return(0); } diff --git a/src/cmd/ksh93/features/externs b/src/cmd/ksh93/features/externs index f503564ff..e1e2b8089 100644 --- a/src/cmd/ksh93/features/externs +++ b/src/cmd/ksh93/features/externs @@ -46,7 +46,7 @@ tst note{ determining extra bytes per argument for arguments list }end output{ for(i=0; environ[i]; i++) envlen += strlen(environ[i]) + 1 + extra_bytes; envlen += 1 + extra_bytes; /* final null element */ - argmax = strtoimax(astconf("ARG_MAX",NiL,NiL),NiL,0) - envlen; + argmax = (int)astconf_long(CONF_ARG_MAX) - envlen; if (argmax < 2048) { error(ERROR_ERROR|2, "argmax too small"); diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c index 407833b50..7bf075863 100644 --- a/src/cmd/ksh93/sh/init.c +++ b/src/cmd/ksh93/sh/init.c @@ -144,9 +144,6 @@ char e_version[] = "\n@(#)$Id: Version " extern char **environ; #endif -#undef getconf -#define getconf(x) strtol(astconf(x,NiL,NiL),NiL,0) - struct seconds { Namfun_t hdr; @@ -1223,9 +1220,9 @@ Shell_t *sh_init(register int argc,register char *argv[], Shinit_f userinit) shgd->euserid=geteuid(); shgd->groupid=getgid(); shgd->egroupid=getegid(); - shgd->lim.clk_tck = getconf("CLK_TCK"); - shgd->lim.arg_max = getconf("ARG_MAX"); - shgd->lim.child_max = getconf("CHILD_MAX"); + shgd->lim.arg_max = astconf_long(CONF_ARG_MAX); + shgd->lim.child_max = (int)astconf_long(CONF_CHILD_MAX); + shgd->lim.clk_tck = (int)astconf_long(CONF_CLK_TCK); if(shgd->lim.arg_max <=0) shgd->lim.arg_max = ARG_MAX; if(shgd->lim.child_max <=0) diff --git a/src/cmd/ksh93/sh/io.c b/src/cmd/ksh93/sh/io.c index c8c7dbfe3..db1fa122f 100644 --- a/src/cmd/ksh93/sh/io.c +++ b/src/cmd/ksh93/sh/io.c @@ -403,7 +403,7 @@ int sh_iovalidfd(Shell_t *shp, int fd) return(0); if(fd < shp->gd->lim.open_max) return(1); - max = strtol(astconf("OPEN_MAX",NiL,NiL),NiL,0); + max = (int)astconf_long(CONF_OPEN_MAX); if(fd >= max) { errno = EBADF; diff --git a/src/cmd/ksh93/sh/suid_exec.c b/src/cmd/ksh93/sh/suid_exec.c index 5af7a97a7..f2d61b20a 100644 --- a/src/cmd/ksh93/sh/suid_exec.c +++ b/src/cmd/ksh93/sh/suid_exec.c @@ -329,7 +329,7 @@ int eaccess(register const char *name, register int mode) if((maxgroups=getgroups(0,groups)) < 0) { /* pre-POSIX system */ - maxgroups=NGROUPS_MAX; + maxgroups = (int)astconf_long(CONF_NGROUPS_MAX); } } groups = (gid_t*)malloc((maxgroups+1)*sizeof(gid_t)); diff --git a/src/lib/libast/comp/eaccess.c b/src/lib/libast/comp/eaccess.c index 1f7d3a039..e83ab373e 100644 --- a/src/lib/libast/comp/eaccess.c +++ b/src/lib/libast/comp/eaccess.c @@ -110,7 +110,7 @@ eaccess(const char* path, register int flags) if (ngroups == -2) { if ((ngroups = getgroups(0, (gid_t*)0)) <= 0) - ngroups = NGROUPS_MAX; + ngroups = (int)astconf_long(CONF_NGROUPS_MAX); if (!(groups = newof(0, gid_t, ngroups + 1, 0))) ngroups = -1; else diff --git a/src/lib/libast/include/ast.h b/src/lib/libast/include/ast.h index 3cac45443..9093e1f51 100644 --- a/src/lib/libast/include/ast.h +++ b/src/lib/libast/include/ast.h @@ -283,6 +283,27 @@ extern off_t astcopy(int, int, off_t); extern int astlicense(char*, int, char*, char*, int, int, int); extern int astquery(int, const char*, ...); extern void astwinsize(int, int*, int*); +#if _lib_sysconf +/* prefer sysconf for astconf_long and astconf_ulong to improve performance */ +#define CONF_ARG_MAX _SC_ARG_MAX +#define CONF_CHILD_MAX _SC_CHILD_MAX +#define CONF_CLK_TCK _SC_CLK_TCK +#define CONF_NGROUPS_MAX _SC_NGROUPS_MAX +#define CONF_OPEN_MAX _SC_OPEN_MAX +#define CONF_PAGESIZE _SC_PAGESIZE +#define astconf_long(x) sysconf(x) +#define astconf_ulong(x) (unsigned long)sysconf(x) +#else +/* fallback in case sysconf isn't available */ +#define CONF_ARG_MAX "ARG_MAX" +#define CONF_CHILD_MAX "CHILD_MAX" +#define CONF_CLK_TCK "CLK_TCK" +#define CONF_NGROUPS_MAX "NGROUPS_MAX" +#define CONF_OPEN_MAX "OPEN_MAX" +#define CONF_PAGESIZE "PAGESIZE" +#define astconf_long(x) strtol(astconf(x,NiL,NiL),NiL,0) +#define astconf_ulong(x) strtoul(astconf(x,NiL,NiL),NiL,0) +#endif extern ssize_t base64encode(const void*, size_t, void**, void*, size_t, void**); extern ssize_t base64decode(const void*, size_t, void**, void*, size_t, void**); diff --git a/src/lib/libast/man/ast.3 b/src/lib/libast/man/ast.3 index 4e6a7b082..40dfb7e96 100644 --- a/src/lib/libast/man/ast.3 +++ b/src/lib/libast/man/ast.3 @@ -44,6 +44,8 @@ ast \- miscellaneous libast support #include char* astconf(const char* \fIname\fP, const char* \fIpath\fP, const char* \fIvalue\fP); +long astconf_long(\fIarg\fP); +unsigned long astconf_ulong(\fIarg\fP); Ast_confdisc_t astconfdisc(Ast_confdisc_t new_notify); void astconflist(Sfio_t* stream, const char* path, int flags); off_t astcopy(int \fIrfd\fP, int \fIwfd\fP, off_t \fIn\fP); @@ -155,6 +157,30 @@ User defined values may also be set and queried, but these should probably have some form of vendor prefix to avoid being stomped by future standards. .PP +.L astconf_long +and +.L astconf_ulong +are macros that call either +.I sysconf +or +.LR astconf , +preferring +.I sysconf +if it's available. +.L astconf_long +returns a signed long value while +.L astconf_ulong +returns an unsigned long value. +The argument passed to these macros may be one of the following: +.EX +CONF_ARG_MAX +CONF_CHILD_MAX +CONF_CLK_TCK +CONF_NGROUPS_MAX +CONF_OPEN_MAX +CONF_PAGESIZE +.EE +.PP .L astconfdisc registers a discipline function .EX @@ -259,7 +285,7 @@ and are set to .LR 0 . If -.I ioctl (2) +.IR ioctl (2) methods fail then the environment variable .L LINES is used to set diff --git a/src/lib/libast/misc/cmdarg.c b/src/lib/libast/misc/cmdarg.c index 88d4896e8..7c5e205dd 100644 --- a/src/lib/libast/misc/cmdarg.c +++ b/src/lib/libast/misc/cmdarg.c @@ -128,7 +128,7 @@ cmdopen_20120411(char** argv, int argmax, int size, const char* argpat, Cmddisc_ argc = 0; for (p = environ; *p; p++) n += sizeof(char**) + strlen(*p) + 1; - if ((x = strtol(astconf("ARG_MAX", NiL, NiL), NiL, 0)) <= 0) + if ((x = astconf_long(CONF_ARG_MAX)) <= 0) x = ARG_MAX; if (size <= 0 || size > x) size = x; diff --git a/src/lib/libast/path/pathtemp.c b/src/lib/libast/path/pathtemp.c index a8e4cea66..8ba986b06 100644 --- a/src/lib/libast/path/pathtemp.c +++ b/src/lib/libast/path/pathtemp.c @@ -90,7 +90,7 @@ static inline int xaccess(const char *path, int mode) int ret; if (!pgsz) - pgsz = strtoul(astconf("PAGESIZE",NiL,NiL),NiL,0); + pgsz = astconf_ulong(CONF_PAGESIZE); if (!path || !*path) { diff --git a/src/lib/libast/stdio/vfwscanf.c b/src/lib/libast/stdio/vfwscanf.c index 2b23dfeea..8a99d0a6a 100644 --- a/src/lib/libast/stdio/vfwscanf.c +++ b/src/lib/libast/stdio/vfwscanf.c @@ -92,7 +92,7 @@ vfwscanf(Sfio_t* f, const wchar_t* fmt, va_list args) n = wcstombs(NiL, fmt, 0); if (w = newof(0, Wide_t, 1, n)) { - if (t = sfnew(NiL, buf, sizeof(buf), OPEN_MAX+1, SF_READ)) + if (t = sfnew(NiL, buf, sizeof(buf), (int)astconf_long(CONF_OPEN_MAX)+1, SF_READ)) { w->sfdisc.exceptf = wideexcept; w->sfdisc.readf = wideread; diff --git a/src/lib/libast/string/fmtclock.c b/src/lib/libast/string/fmtclock.c index c9d7c7e3c..9cb7d5c17 100644 --- a/src/lib/libast/string/fmtclock.c +++ b/src/lib/libast/string/fmtclock.c @@ -36,16 +36,16 @@ fmtclock(register Sfulong_t t) char* buf; int z; - static unsigned int clk_tck; + static unsigned long clk_tck; if (!clk_tck) { #ifdef CLOCKS_PER_SEC clk_tck = CLOCKS_PER_SEC; #else - if (!(clk_tck = (unsigned int)strtoul(astconf("CLK_TCK", NiL, NiL), NiL, 10))) + if (!(clk_tck = astconf_ulong(CONF_CLK_TCK))) clk_tck = 60; -#endif +#endif /* CLOCKS_PER_SEC */ } if (t == 0) return "0"; diff --git a/src/lib/libast/vmalloc/malloc.c b/src/lib/libast/vmalloc/malloc.c index 94afafc2c..386b5f761 100644 --- a/src/lib/libast/vmalloc/malloc.c +++ b/src/lib/libast/vmalloc/malloc.c @@ -1124,8 +1124,6 @@ char* ends; return begs; } -#define FD_PRIVATE (3*OPEN_MAX/4) - #if __STD_C int _vmfd(int fd) #else @@ -1134,10 +1132,19 @@ int fd; #endif { int pd; + int fd_private = (int)(3 * astconf_long(CONF_OPEN_MAX) / 4); + if (fd_private <= 0) + { +#if defined(OPEN_MAX) + fd_private = 3 * OPEN_MAX / 4; +#else + fd_private = 3 * FOPEN_MAX / 4; +#endif + } if (fd >= 0) { - if (fd < FD_PRIVATE && (pd = fcntl(fd, F_DUPFD, FD_PRIVATE)) >= 0) + if (fd < fd_private && (pd = fcntl(fd, F_DUPFD, fd_private)) >= 0) { close(fd); fd = pd; diff --git a/src/lib/libcmd/fds.c b/src/lib/libcmd/fds.c index 81a864a00..94d824c30 100644 --- a/src/lib/libcmd/fds.c +++ b/src/lib/libcmd/fds.c @@ -54,9 +54,6 @@ static const char usage[] = #define major(x) (int)(((unsigned int)(x)>>8)&0xff) #endif -#undef getconf -#define getconf(x) strtol(astconf(x,NiL,NiL),NiL,0) - #ifdef S_IFSOCK typedef struct NV_s @@ -214,8 +211,14 @@ b_fds(int argc, char** argv, Shbltin_t* context) error(ERROR_usage(2), "%s", optusage(NiL)); UNREACHABLE(); } - if ((open_max = getconf("OPEN_MAX")) <= 0) + if ((open_max = (int)astconf_long(CONF_OPEN_MAX)) <= 0) + { +#if defined(OPEN_MAX) open_max = OPEN_MAX; +#else + open_max = FOPEN_MAX; +#endif + } if (unit == 1) sp = sfstdout; else if (fstat(unit, &st) || !(sp = sfnew(NiL, NiL, SF_UNBOUND, unit, SF_WRITE))) diff --git a/src/lib/libcmd/id.c b/src/lib/libcmd/id.c index 398bf6198..fb14d9fb4 100644 --- a/src/lib/libcmd/id.c +++ b/src/lib/libcmd/id.c @@ -231,7 +231,7 @@ getids(Sfio_t* sp, const char* name, register int flags) */ if ((maxgroups = getgroups(0, groups)) <= 0) - maxgroups = NGROUPS_MAX; + maxgroups = (int)astconf_long(CONF_NGROUPS_MAX); if (!(groups = newof(0, gid_t, maxgroups + 1, 0))) { error(ERROR_SYSTEM|ERROR_PANIC, "out of memory [group array]");