public inbox for git-commits@fedoraproject.org
help / color / mirror / Atom feed
From: Paulo Alcantara <paalcant@redhat.com>
To: git-commits@fedoraproject.org
Subject: [rpms/cifs-utils] f43: cifs.upcall: fix regression with kerberos mounts
Date: Mon, 06 Jul 2026 22:15:14 GMT [thread overview]
Message-ID: <178337611490.1.15418076230128843032.rpms-cifs-utils-a054f54b8530@fedoraproject.org> (raw)
A new commit has been pushed.
Repo : rpms/cifs-utils
Branch : f43
Commit : a054f54b8530b3fbaedfbc9821f23991d82143c5
Author : Paulo Alcantara <paalcant@redhat.com>
Date : 2026-07-06T19:04:37-03:00
Stats : +392/-0 in 2 file(s)
URL : https://src.fedoraproject.org/rpms/cifs-utils/c/a054f54b8530b3fbaedfbc9821f23991d82143c5?branch=f43
Log:
cifs.upcall: fix regression with kerberos mounts
- Resolves: rhbz#2497446
- Resolves: rhbz#2496963
- Resolves: rhbz#2489808 - CVE-2026-12505
Signed-off-by: Paulo Alcantara <paalcant@redhat.com>
---
diff --git a/cifs-utils.spec b/cifs-utils.spec
index eb354cf..487ff2e 100644
--- a/cifs-utils.spec
+++ b/cifs-utils.spec
@@ -25,6 +25,7 @@ Recommends: %{name}-info%{?_isa} = %{version}-%{release}
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..446c39b
--- /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 <ematsumiya@suse.de>
+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) <pc@manguebit.org>
+Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
+Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
+Signed-off-by: Steve French <stfrench@microsoft.com>
+---
+ 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 <arpa/inet.h>
+ #include <ctype.h>
+ #include <grp.h>
++#include <pwd.h>
+ #include <stdbool.h>
+ #include <errno.h>
+ #include <sched.h>
+@@ -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:
++ * <NS ID start> <host ID start> <range>
++ * ... (<multiple entries are supported) ...
++ *
++ * Formula and validation:
++ * <final NS ID> = (*@idp - <host ID start>) + <NS ID start>
++ *
++ * - 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:
+- * <real> <effective> <saved> <fsuid>
++ * 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 <real> 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.54.0
+
reply other threads:[~2026-07-06 22:15 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=178337611490.1.15418076230128843032.rpms-cifs-utils-a054f54b8530@fedoraproject.org \
--to=paalcant@redhat.com \
--cc=git-commits@fedoraproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox