From 8abf6d7ae20a02487189be6bb6acf0d4ba4a5043 Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Tue, 7 Jul 2026 15:54:30 -0300 Subject: [PATCH] resolves: RHEL-192934 - fix krb5 mount regression Signed-off-by: Paulo Alcantara --- cifs-utils.spec | 1 + ...all-fix-regression-with-krb5-creduid.patch | 391 ++++++++++++++++++ 2 files changed, 392 insertions(+) create mode 100644 cifs.upcall-fix-regression-with-krb5-creduid.patch diff --git a/cifs-utils.spec b/cifs-utils.spec index 38fce50..fdb3975 100644 --- a/cifs-utils.spec +++ b/cifs-utils.spec @@ -23,6 +23,7 @@ Requires(preun): /usr/sbin/alternatives Source0: https://download.samba.org/pub/linux-cifs/cifs-utils/%{name}-%{version}.tar.bz2 Patch0: cifs.upcall-fix-compiler-warning-with-Wvla.patch +Patch1: cifs.upcall-fix-regression-with-krb5-creduid.patch %description The SMB/CIFS protocol is a standard file sharing protocol widely deployed diff --git a/cifs.upcall-fix-regression-with-krb5-creduid.patch b/cifs.upcall-fix-regression-with-krb5-creduid.patch new file mode 100644 index 0000000..6e74e13 --- /dev/null +++ b/cifs.upcall-fix-regression-with-krb5-creduid.patch @@ -0,0 +1,391 @@ +From e9495963e0d5c26f7d0137829d8dc625130b53cc Mon Sep 17 00:00:00 2001 +From: Enzo Matsumiya +Date: Mon, 6 Jul 2026 10:55:55 -0300 +Subject: [PATCH] cifs.upcall: fix regression with krb5 + creduid + +Commit 972c5b5ff95e ("cifs.upcall: remove getpwuid() dependency") +introduced a regression when using creduid != uid (e.g. +"mount.cifs -o sec=krb5,cruid=X"), so 'uid' local var is replaced +with procfs "Uid" value (in the example, the one from mount.cifs). + +That commit ignored the fact that: + mount UID can be different from creds UID, and that calling-app + process (post-mount) can be different from both + +This patch "reverts" 972c5b5ff95e ("cifs.upcall: remove getpwuid() +dependency"); transform the "emergency"-added function get_uidgid() +into map_uidgid(), now only used to do NS UID/GID mapping. + +Also add getpwuid() back, but this time called while still on host +namespace, so any possible custom NSS module is ran as allowed by +sysadmin. + +Any scenario involving unmapped UIDs or GIDs is unsupported; this +means that any UID:GID in a child user namespace _must_ map back to +a valid and existing host UID:GID. + +Fixes: 972c5b5ff95e ("cifs.upcall: remove getpwuid() dependency") +Reported-by: Paulo Alcantara (Red Hat) +Signed-off-by: Enzo Matsumiya +Signed-off-by: Paulo Alcantara (Red Hat) +Signed-off-by: Steve French +--- + cifs.upcall.c | 279 ++++++++++++++++++++++++++++++++------------------ + 1 file changed, 177 insertions(+), 102 deletions(-) + +diff --git a/cifs.upcall.c b/cifs.upcall.c +index 747617790576..42205e66a676 100644 +--- a/cifs.upcall.c ++++ b/cifs.upcall.c +@@ -52,6 +52,7 @@ + #include + #include + #include ++#include + #include + #include + #include +@@ -1375,118 +1376,170 @@ static int ip_to_fqdn(const char *addrstr, char *host, size_t hostlen) + return 0; + } + +-/* cover worst case/impossible scenarios */ +-#define PROC_PID_PATH_MAXLEN ((int)sizeof("/proc/2147483647/status")) +-/* max valid UID/GID is (UINT_MAX - 1) */ +-#define INVALID_UIDGID UINT_MAX ++#define ID_MAP_PATH_MAX ((int)sizeof("/proc/2147483647/uid_map")) + +-/* +- * get_uidgid - Get @pid's (real) UID and/or GID. +- * @pid: process to get UID/GID from +- * @uidp: pointer to store @pid's UID (can be NULL) +- * @gidp: pointer to store @pid's GID (can be NULL) +- * +- * Extract "Uid:" and "Gid:" fields from /proc/@pid/status. +- * Do so based on whether @uidp or @gidp are NULL. +- * +- * This function assumes we're on the same namespace as @pid. +- * +- * Return: 0 on success, -1 otherwise (errno set). +- * +- * On errors, *@uidp and *@gidp are set to INVALID_UIDGID. +- */ +-static int get_uidgid(pid_t pid, uid_t *uidp, gid_t *gidp) ++static int map_id(pid_t pid, const char *map, unsigned int *idp) + { +- char path[PROC_PID_PATH_MAXLEN] = {}, buf[256]; ++ unsigned long long ns_start, host_start, range; ++ char map_path[ID_MAP_PATH_MAX]; ++ int map_path_size = sizeof(map_path); ++ unsigned int id; + FILE *fp = NULL; +- int ret; ++ int ret = 1; + +- errno = 0; +- if (pid < 0 || (!uidp && !gidp)) { +- errno = EINVAL; +- return -1; +- } ++ errno = EINVAL; ++ if (pid < 0 || !map || !idp || *idp == UINT_MAX) ++ goto out; + +- if (uidp) +- *uidp = INVALID_UIDGID; +- +- if (gidp) +- *gidp = INVALID_UIDGID; +- +- ret = snprintf(path, PROC_PID_PATH_MAXLEN, "/proc/%d/status", pid); +- if (ret < 0 || ret >= PROC_PID_PATH_MAXLEN) { +- if (!errno) ++ ret = snprintf(map_path, map_path_size, "/proc/%d/%s", pid, map); ++ if (ret < 0 || ret >= map_path_size) { ++ if (ret >= map_path_size) + errno = ENAMETOOLONG; +- return -1; ++ ret = 1; ++ goto out; + } + +- fp = fopen(path, "r"); +- if (!fp) { +- ret = -1; ++ ret = 1; ++ fp = fopen(map_path, "r"); ++ if (!fp) + goto out; +- } + +- /* Parse /proc/pid/status fields */ +- errno = 0; +- ret = -1; +- while (fgets(buf, 256, fp)) { +- unsigned long long val; ++ /* ++ * The map files have the same format: ++ * ++ * ... ( = (*@idp - ) + ++ * ++ * - IDs: [0, UINT_MAX - 1] ++ * - range: [1, UINT_MAX], where range == UINT_MAX requires both ID ranges to start at 0, ++ * which then means this is an init host NS mapping (and that's ok) ++ * ++ * The formula itself would be enough to "validate" a matching NS ID, but we don't want to ++ * keep parsing a malformed map file, no matter how unlikely/impossible it is to happen. ++ * Same reason values are parsed as 'unsigned long long', so we can check for bogus data. ++ */ ++ id = UINT_MAX; ++ errno = ENODATA; ++ while (fscanf(fp, "%llu %llu %llu", &ns_start, &host_start, &range) == 3) { ++ if (ns_start >= UINT_MAX || host_start >= UINT_MAX || ++ range > UINT_MAX || range == 0) { ++ errno = EINVAL; ++ ret = 1; ++ break; ++ } + +- errno = ENODATA; +- if ((!uidp || strncmp(buf, "Uid:", 4)) && (!gidp || strncmp(buf, "Gid:", 4))) +- continue; ++ if (range == UINT_MAX && (ns_start != 0 || host_start != 0)) { ++ errno = EINVAL; ++ ret = 1; ++ break; ++ } + +- errno = 0; ++ if (host_start + range > UINT_MAX || ns_start + range > UINT_MAX) { ++ errno = EINVAL; ++ ret = 1; ++ break; ++ } + + /* +- * Example line format (same for both Uid/Gid): +- * "Uid:\t%u\t%u\%u\%u" ++ * Check if host ID fits this line. ++ * Our desired NS ID may be in any line of the file. + * +- * Where the numbers represents: +- * ++ * Note: new{uid,gid}map tools (that creates the map files) don't allow multiple ++ * maps (NS IDs) to the same host ID. + * +- * We're only interested in the value. ++ * If we get a match here, we'll save it, but we continue parsing the file. ++ * If we happen to find a duplicate, it's possible this is a rogue file trying to ++ * bypass these checks. + * +- * (field names "Uid:"/"Gid:" parsed above, skip it) ++ * In such cases, discard the match and return EOPNOTSUPP, as returning a ++ * successful match could lead to disastrous results. + */ +- ret = sscanf(&buf[0] + 4, "%llu", &val); +- if (ret != 1) { +- ret = -1; +- if (errno) ++ if (*idp >= host_start && *idp < host_start + range) { ++ /* This means we found a duplicate */ ++ if (!ret) { ++ ret = 1; ++ errno = EOPNOTSUPP; ++ syslog(LOG_ERR, "%s has multiple mapped IDs for %u (unsupported)", ++ map, *idp); + break; +- continue; +- } ++ } + +- ret = -1; +- if (val >= UINT_MAX) { +- errno = EINVAL; +- break; +- } +- +- if (uidp && !strncmp(buf, "Uid:", 4)) +- *uidp = (uid_t)val; +- else +- *gidp = (gid_t)val; +- +- if ((!uidp || *uidp != INVALID_UIDGID) && (!gidp || *gidp != INVALID_UIDGID)) { +- errno = 0; ++ id = (*idp - host_start) + ns_start; + ret = 0; +- break; ++ errno = 0; + } + } ++ ++ /* This means errno was reset by fscanf() without finding anything */ ++ if (ret && errno == 0) ++ errno = ENODATA; + out: +- if (fp) ++ if (fp) { ++ int err = errno; ++ + fclose(fp); ++ /* Ignore fclose() errors */ ++ errno = err; ++ } + ++ if (!ret) { ++ *idp = id; ++ errno = 0; ++ } else { ++ syslog(LOG_DEBUG, "%s(pid=%d, map=%s, id=%u): %s", __func__, pid, map, *idp, ++ strerror(errno)); ++ } ++ ++ return ret; ++} ++ ++/* ++ * map_uidgid() - Map (real) UID/GID from init host NS to user NS. ++ * @pid: host NS PID ++ * @uidp: (in) host UID, (out) NS UID ++ * @gidp: (in) host GID, (out) NS GID ++ * ++ * Parse /proc/@pid/{uid,gid}_map files to get NS UID/GID values. ++ * Since @pid is expected to be a host NS PID, this must be called before switching namespaces. ++ * ++ * Note: we can't use /proc/self here because we haven't switched NS yet, so {uid,gid}_map files ++ * would contain host NS values. ++ * ++ * Return: 0 on success, 1 otherwise (errno set). ++ */ ++static int map_uidgid(pid_t pid, uid_t *uidp, gid_t *gidp) ++{ ++ uid_t orig_uid; ++ gid_t orig_gid; ++ int ret = 1; ++ ++ errno = EINVAL; ++ if (!uidp || !gidp) ++ goto out; ++ ++ orig_uid = *uidp; ++ orig_gid = *gidp; ++ ++ ret = map_id(pid, "uid_map", uidp); + if (ret) { +- syslog(LOG_DEBUG, "%s(pid=%d): %s", __func__, pid, strerror(errno)); +- if (uidp) +- *uidp = INVALID_UIDGID; +- +- if (gidp) +- *gidp = INVALID_UIDGID; ++ if (errno == ENODATA) ++ syslog(LOG_ERR, "UID %u not mapped in this namespace (unsupported)", ++ orig_uid); ++ goto out; + } + ++ ret = map_id(pid, "gid_map", gidp); ++ if (ret && errno == ENODATA) ++ syslog(LOG_ERR, "GID %u not mapped in this namespace (unsupported)", ++ orig_gid); ++out: ++ if (ret && errno != ENODATA) ++ syslog(LOG_ERR, "%s: %s", __func__, strerror(errno)); ++ else if (!ret) ++ syslog(LOG_DEBUG, "host %u:%u -> NS %u:%u", orig_uid, orig_gid, *uidp, *gidp); ++ + return ret; + } + +@@ -1534,6 +1587,7 @@ int main(const int argc, char *const argv[]) + const char *oid; + uid_t uid; + gid_t gid; ++ struct passwd *pw; + char *keytab_name = NULL; + char *env_cachename = NULL; + krb5_ccache ccache = NULL; +@@ -1680,12 +1734,52 @@ int main(const int argc, char *const argv[]) + goto out; + } + ++ /* ++ * 'uid' always points to a host UID, so we must get the corresponding host GID. ++ * It's safe to call getpwuid() here because we're still on host NS, i.e. caller ++ * application has no control over custom NSS modules. ++ * ++ * FIXME: if UID is from another NS, or a subuid, this will fail on mount. ++ */ ++ errno = 0; ++ pw = getpwuid(uid); ++ if (!pw) { ++ syslog(LOG_ERR, "failed to retrieve GID from UID %u: %s", uid, ++ strerror(errno ? errno : ENOENT)); ++ rc = 1; ++ goto out; ++ } ++ ++ gid = pw->pw_gid; ++ ++ /* ++ * We can't reasonably do this for root. When mounting a DFS share, ++ * for instance we can end up with creds being overridden, but the env ++ * variable left intact. ++ * ++ * Always check this before NS UID mapping. ++ */ ++ if (uid == 0) ++ env_probe = false; ++ + /* + * Change to the process's namespace. This means that things will work + * acceptably in containers, because we'll be looking at the correct + * filesystem and have the correct network configuration. + */ + if (arg->upcall_target == UPTARGET_APP || arg->upcall_target == UPTARGET_UNSPECIFIED) { ++ /* ++ * Map host 'uid' and 'gid' to the target user NS. ++ * ++ * Any scenario that involves unmapped UIDs or primary GIDs is not supported -- we ++ * don't have, and can't find, all the info that would be necessary to find a ++ * UID/GID within all the possible NS combinations. ++ */ ++ if (!in_same_user_ns(arg->pid, getpid())) { ++ rc = map_uidgid(arg->pid, &uid, &gid); ++ if (rc) ++ goto out; ++ } + syslog(LOG_INFO, "upcall_target=app, switching namespaces to application thread"); + arg->upcall_target = UPTARGET_APP; + rc = switch_to_process_ns(arg->pid); +@@ -1700,25 +1794,6 @@ int main(const int argc, char *const argv[]) + syslog(LOG_INFO, "upcall_target=mount, not switching namespaces to application thread"); + } + +- /* +- * We can't reasonably do this for root. When mounting a DFS share, +- * for instance we can end up with creds being overridden, but the env +- * variable left intact. +- */ +- if (uid == 0) +- env_probe = false; +- +- /* +- * FIXME: this only works if we haven't switched PID namespaces. +- * If we did, /proc/arg->pid/ might not exist, or worse, point to something else. +- */ +- rc = get_uidgid(arg->pid, &uid, &gid); +- if (rc) { +- syslog(LOG_ERR, "get_uidgid (NS): %s", strerror(errno)); +- rc = 1; +- goto out; +- } +- + rc = setgid(gid); + if (rc) { + syslog(LOG_ERR, "setgid: %s", strerror(errno)); +-- +2.55.0 +