From 6a0f960379bf0d5acbdc5ce8b2aa935a40c584af Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Thu, 27 Aug 2026 17:09:00 +0200 Subject: [PATCH] mxqd: Refactor _getpwuid Rename _getpwuid to _getpw and redesign its API. --- mxqd.c | 76 ++++++++++++++++++++++++++++++---------------------------- 1 file changed, 39 insertions(+), 37 deletions(-) diff --git a/mxqd.c b/mxqd.c index 29b2d05..97c7e1b 100644 --- a/mxqd.c +++ b/mxqd.c @@ -893,38 +893,41 @@ static int is_reaper(pid_t pid) { return 0; } -static struct passwd *_getpwuid(uid_t uid, const char *name, struct passwd *pwbuf, char **bufp) { - long bufsize_hint = sysconf(_SC_GETPW_R_SIZE_MAX); - size_t bufsize = bufsize_hint > 0 ? (size_t)bufsize_hint : 1024; - - for (;;) { - char *buf = malloc(bufsize); - if (!buf) - return NULL; - - struct passwd *result; - int res = name ? getpwnam_r(name, pwbuf, buf, bufsize, &result) - : getpwuid_r(uid, pwbuf, buf, bufsize, &result); - - if (res == 0) { - if (!result) { - free(buf); - errno = 0; - return NULL; - } - *bufp = buf; - return pwbuf; - } +// success: returns 0. *out is valid and needs to be freed by caller +// success but not found: returns 0, *out is set to NULL +// failure: returns error number, *out is set to NULL +// +static int _getpw(uid_t uid, const char *name, struct passwd **out) { + // sysconf(_SC_GETPW_R_SIZE_MAX) is 1024 but a typical user only needs ~80 bytes. + // so don't bother with _SC_GETPW_R_SIZE_MAX + // just start with 2 * 80 = 160. + // note sizeof (struct passwd) is 48 + + size_t bufsize = 160; - free(buf); - if (res != ERANGE) { - errno = res; - return NULL; + while (1) { + char *block = malloc(sizeof(struct passwd) + bufsize); + if (!block) { + *out = NULL; + return ENOMEM; } + + struct passwd *pwd = (struct passwd *)block; + char *buf = block + sizeof(struct passwd); + + int res = name ? getpwnam_r(name, pwd, buf, bufsize, out) + : getpwuid_r(uid, pwd, buf, bufsize, out); + if (*out) + return res; // *out is always set. only if something found that *out != NULL and res==0. + free(block); + if (res != ERANGE) + return res; // 0: success (not found) or an error bufsize *= 2; } } +// intentionally leak memory. Caller is expected to exit if we return. +// static void exec_reaper(struct mxq_server *server,struct mxq_group_list *glist, struct mxq_job *job) { struct mxq_group *group = &glist->group; @@ -944,10 +947,10 @@ static void exec_reaper(struct mxq_server *server,struct mxq_group_list *glist, sigprocmask(SIG_UNBLOCK,&all_signals,NULL); signal(SIGPIPE,SIG_DFL); - _mx_cleanup_free_ char *pwbuf = NULL; - struct passwd passwd_buf; - struct passwd *passwd = _getpwuid(group->user_uid, NULL, &passwd_buf, &pwbuf); + struct passwd *passwd; // initialized in next call, intentionally leaked + int res = _getpw(group->user_uid, NULL, &passwd); if (!passwd) { + errno = res ? res : ENOENT; mx_log_err("job=%s(%d):%lu:%lu getpwuid_r(): %m", group->user_name, group->user_uid, group->group_id, job->job_id); return; @@ -963,15 +966,15 @@ static void exec_reaper(struct mxq_server *server,struct mxq_group_list *glist, group->user_name, passwd->pw_name); - free(pwbuf); - pwbuf = NULL; - passwd = _getpwuid(0, group->user_name, &passwd_buf, &pwbuf); - if (!passwd) { + struct passwd *passwd2; // initialized in next call, intentionally leaked + res = _getpw(0, group->user_name, &passwd2); + if (!passwd2) { + errno = res ? res : ENOENT; mx_log_err("job=%s(%d):%lu:%lu getpwnam_r(): %m", group->user_name, group->user_uid, group->group_id, job->job_id); return; } - if (passwd->pw_uid != group->user_uid) { + if (passwd2->pw_uid != group->user_uid) { mx_log_fatal("job=%s(%d):%lu:%lu user_name=%s does not map to uid=%d but to pw_uid=%d. Aborting Child execution.", group->user_name, group->user_uid, @@ -979,7 +982,7 @@ static void exec_reaper(struct mxq_server *server,struct mxq_group_list *glist, job->job_id, group->user_name, group->user_uid, - passwd->pw_uid); + passwd2->pw_uid); return; } @@ -1011,10 +1014,9 @@ static void exec_reaper(struct mxq_server *server,struct mxq_group_list *glist, if (group->job_tmpdir_size == 0) { mx_setenv_forever("TMPDIR", server->initial_tmpdir); } else { - char *mxq_job_tmpdir = mx_asprintf_forever("%s/%lu", MXQ_JOB_TMPDIR_MNTDIR, job->job_id); + char *mxq_job_tmpdir = mx_asprintf_forever("%s/%lu", MXQ_JOB_TMPDIR_MNTDIR, job->job_id); // intentionally leaked mx_setenv_forever("MXQ_JOB_TMPDIR", mxq_job_tmpdir); mx_setenv_forever("TMPDIR", mxq_job_tmpdir); - // not needed before exec() or exit(): free(mxq_job_tmpdir); } if (group->job_gpu) { char *argv[] = {