[PATCH v2 00/13] cred: gather the "same uid" checks between tasks in one place

[PATCH v2 00/13] cred: gather the "same uid" checks between tasks in one place

[Josef Bacik]

v1: https://lore.kernel.org/all/20261018-b4-cred-ids-v1-0-7bdf2db4ea9f@toxicpanda.com/

v1->v2:
- Rebased onto v7.4-rc1, patch 1 didn't apply any more.
- Patch 1: VFS_WARN_ON_ONCE() in the default case of
  cred_ids_allow_control() instead of failing silently (Serge).
- Dropped the Assisted-by trailers, see 1e035297ffda ("docs: remove
  Assisted-by references").

--- Original email (v1) ---

Hello,

We have a dozen places in the kernel that let one task mess with another
because they belong to the same user: kill(), SIGIO, the scheduler
syscalls, setpriority(), ioprio_set(), prlimit(), ptrace, moving a task
around in cgroup v1 and resctrl, and KEYCTL_SESSION_TO_PARENT.  Every
one of them open codes a handful of uid_eq() calls on the two creds.
They come in six flavors that differ in which of the real, effective and
saved ids they look at, and in whether euid 0 gets a pass, and there is
nothing that tells you whether two of them differ on purpose or by
accident.

I tripped over this working on something else that needs to change what
"the same user" means.  I kept finding another copy I had missed, and
for this kind of check a copy you missed is a hole.  So this pulls all
of them into kernel/cred.c, side by side, with a table that says which
rule looks at what, and converts the callers one at a time.

Nothing is supposed to change.  What each of these means by "the same
user" is ABI at this point, so every rule is kept exactly as it was,
warts and all, I have not tried to unify them.  The capability checks
and the LSM hooks stay with the callers.

The one patch that isn't a straight substitution is the fcntl one.
F_SETOWN copies the uid and euid of the caller into the fown_struct, and
the helpers take creds, so it holds a reference on the cred instead, the
same way struct file does for the opener.  That means the cred sticks
around until the owner is set again or the file goes away.

For testing, patch 2 is a KUnit test that holds each helper against the
expression it replaces, verbatim, over every combination of a few ids. I
also ran kill, setpriority, ioprio_set, sched_setaffinity,
sched_setscheduler, prlimit, a cgroup v1 move, SIGIO and PTRACE_SEIZE
for all 64 combinations of real, effective and saved uid of the two
tasks, on a kernel with and without the series under KASAN and lockdep.
All 576 results are the same on both, and they match what the table
says.  The resctrl and usbdevfs (kill_pid_usb_asyncio) callers I have no
way to exercise, and the MIPS patch is compile tested only.

This is based on vfs.all only because that's what the other work sits
on. Thanks,

Josef

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
Changes in v2:
- Link to v1: https://patch.msgid.link/20261018-b4-cred-ids-v1-0-7bdf2db4ea9f@toxicpanda.com

---
Josef Bacik (13):
      cred: gather the rules for acting on a task by uid
      cred: add KUnit tests for the id rules
      signal: use cred_ids_allow_signal()
      fcntl: use cred_ids_allow_sigio()
      sched: use cred_ids_allow_sched()
      MIPS: use cred_ids_allow_sched()
      sys: use cred_ids_allow_sched() for setpriority()
      sys: use cred_ids_allow_control() for prlimit()
      block: use cred_ids_allow_ioprio()
      cgroup/v1: use cred_ids_allow_move()
      fs/resctrl: use cred_ids_allow_move()
      ptrace: use cred_ids_allow_control()
      keys: use cred_ids_allow_control()

 Documentation/security/credentials.rst |  19 +++
 MAINTAINERS                            |   1 +
 arch/mips/kernel/mips-mt-fpaff.c       |   3 +-
 block/blk-ioc.c                        |   3 +-
 fs/fcntl.c                             |  19 +--
 fs/resctrl/rdtgroup.c                  |   4 +-
 include/linux/cred.h                   |  20 +++
 include/linux/fs.h                     |   4 +-
 kernel/Makefile                        |   1 +
 kernel/cgroup/cgroup-v1.c              |   4 +-
 kernel/cred.c                          | 162 ++++++++++++++++++++++++
 kernel/cred_kunit.c                    | 218 +++++++++++++++++++++++++++++++++
 kernel/ptrace.c                        |  16 +--
 kernel/sched/syscalls.c                |   3 +-
 kernel/signal.c                        |  10 +-
 kernel/sys.c                           |  10 +-
 lib/Kconfig.debug                      |  13 ++
 security/keys/keyctl.c                 |   7 +-
 security/landlock/fs.h                 |   3 +-
 19 files changed, 461 insertions(+), 59 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261018-b4-cred-ids-eb01b2b38524

[PATCH v2 01/13] cred: gather the rules for acting on a task by uid

[Josef Bacik]

A number of operations let one task act on another without any capability,
because the two belong to the same user: signals, scheduling attributes,
priorities, resource limits, ptrace and more. Each of them compares the
ids of the two credentials by hand. There are twelve such places, in six
flavours that differ in which ids they look at, and nothing says that they
belong together.

That makes them hard to review and hard to change. Nobody can tell from
one of them whether it differs from its neighbour on purpose. Whoever
wants to know what an equal uid is good for has to find all twelve, and so
does whoever has to change it. A place that is overlooked is a hole.

Gather the rules in kernel/cred.c, side by side:

 rule     of the subject    of the object      euid 0 used for
 signal   euid or uid       suid or uid        no     kill()
 sigio    euid or uid       suid or uid        yes    SIGIO, SIGURG
 sched    euid              euid or uid        no     scheduler, nice
 ioprio   euid or uid       uid                no     ioprio_set()
 move     euid              uid or suid        yes    cgroup v1, resctrl
 control  one uid, one gid  all three of each  no     ptrace, prlimit, keys

Where it says so, a subject with euid 0 may act on anybody. That is a
capability by another name, from before there were any, and it belongs
with the rest: it is a uid that decides whether one task may act on
another.

What each operation means by "the same user" has grown separately and
cannot be changed now, so every rule is kept as it is. The functions only
speak for the ids. A capability that does as well, and the LSM hook that
goes with the operation, remain with the caller.

The callers are converted one by one in the patches that follow.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 Documentation/security/credentials.rst |  19 ++++
 include/linux/cred.h                   |  20 ++++
 kernel/cred.c                          | 162 +++++++++++++++++++++++++++++++++
 3 files changed, 201 insertions(+)

diff --git a/Documentation/security/credentials.rst b/Documentation/security/credentials.rst
index a39a2a2f67aa..b095c03b4531 100644
--- a/Documentation/security/credentials.rst
+++ b/Documentation/security/credentials.rst
@@ -422,6 +422,25 @@ magic.  This may not be used for pointer members as what they point to may
 disappear the moment the RCU read lock is dropped.
 
 
+Comparing the Ids of Two Tasks
+------------------------------
+
+Whether one task may act on another because both belong to the same user must not
+be decided by comparing the ids of their credentials by hand.  The rules for that
+are kept in one place:
+
+.. kernel-doc:: kernel/cred.c
+   :doc: One task acting on another on the strength of its ids
+
+.. kernel-doc:: kernel/cred.c
+   :identifiers: cred_ids_allow_signal cred_ids_allow_sigio
+                 cred_ids_allow_sched cred_ids_allow_ioprio
+                 cred_ids_allow_move cred_ids_allow_control
+
+.. kernel-doc:: include/linux/cred.h
+   :identifiers: cred_ids
+
+
 Altering Credentials
 --------------------
 
diff --git a/include/linux/cred.h b/include/linux/cred.h
index 49c26af37349..7fb1663a5fc6 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -171,6 +171,26 @@ extern int cred_fscmp(const struct cred *, const struct cred *);
 extern void __init cred_init(void);
 extern int set_cred_ucounts(struct cred *);
 
+/**
+ * enum cred_ids - which of its ids a subject acts with
+ * @CRED_IDS_REAL:      uid and gid
+ * @CRED_IDS_EFFECTIVE: euid and egid
+ * @CRED_IDS_FS:        fsuid and fsgid
+ */
+enum cred_ids {
+	CRED_IDS_REAL,
+	CRED_IDS_EFFECTIVE,
+	CRED_IDS_FS,
+};
+
+bool cred_ids_allow_signal(const struct cred *subj, const struct cred *obj);
+bool cred_ids_allow_sigio(const struct cred *subj, const struct cred *obj);
+bool cred_ids_allow_sched(const struct cred *subj, const struct cred *obj);
+bool cred_ids_allow_ioprio(const struct cred *subj, const struct cred *obj);
+bool cred_ids_allow_move(const struct cred *subj, const struct cred *obj);
+bool cred_ids_allow_control(const struct cred *subj, enum cred_ids which,
+			    const struct cred *obj);
+
 static inline bool cap_ambient_invariant_ok(const struct cred *cred)
 {
 	return cap_issubset(cred->cap_ambient,
diff --git a/kernel/cred.c b/kernel/cred.c
index d01df40a9a38..5e10e0bdd11e 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -19,6 +19,7 @@
 #include <linux/binfmts.h>
 #include <linux/cn_proc.h>
 #include <linux/uidgid.h>
+#include <linux/vfsdebug.h>
 
 #if 0
 #define kdebug(FMT, ...)						\
@@ -541,6 +542,167 @@ void __init cred_init(void)
 			      SLAB_NO_MERGE);
 }
 
+/**
+ * DOC: One task acting on another on the strength of its ids
+ *
+ * A number of operations let one task act on another without any capability,
+ * because the two belong to the same user. What "the same user" means has grown
+ * separately for each of them and cannot be changed now. All of it is kept
+ * here, side by side, so that there is one place to read it.
+ *
+ * =======  ================  =================  ======  =====================
+ * Rule     Of the subject    Of the object      euid 0  Used for
+ * =======  ================  =================  ======  =====================
+ * signal   euid or uid       suid or uid        no      kill()
+ * sigio    euid or uid       suid or uid        yes     SIGIO, SIGURG
+ * sched    euid              euid or uid        no      scheduler, nice
+ * ioprio   euid or uid       uid                no      ioprio_set()
+ * move     euid              uid or suid        yes     cgroup v1, resctrl
+ * control  one uid, one gid  all three of each  no      ptrace, prlimit, keys
+ * =======  ================  =================  ======  =====================
+ *
+ * Where it says so, a subject with euid 0 may act on anybody. That is a
+ * capability by another name, from before there were any.
+ *
+ * These functions only speak for the ids. A capability that does as well, and
+ * the LSM hook that goes with the operation, remain with the caller.
+ *
+ * Do not compare the ids of two credentials by hand to decide whether one task
+ * may act on another. Use one of these, or add one.
+ */
+
+#define CRED_RUID	BIT(0)
+#define CRED_EUID	BIT(1)
+#define CRED_SUID	BIT(2)
+
+static bool cred_euid_is_root(const struct cred *subj)
+{
+	return uid_eq(subj->euid, GLOBAL_ROOT_UID);
+}
+
+static bool cred_has_uid(const struct cred *cred, unsigned int ids, kuid_t uid)
+{
+	return ((ids & CRED_RUID) && uid_eq(cred->uid, uid)) ||
+	       ((ids & CRED_EUID) && uid_eq(cred->euid, uid)) ||
+	       ((ids & CRED_SUID) && uid_eq(cred->suid, uid));
+}
+
+/*
+ * Is any of the @subj_ids of @subj any of the @obj_ids of @obj? No rule looks at
+ * the saved uid of the subject.
+ */
+static bool cred_uid_match_any(const struct cred *subj, unsigned int subj_ids,
+			       const struct cred *obj, unsigned int obj_ids)
+{
+	return ((subj_ids & CRED_RUID) && cred_has_uid(obj, obj_ids, subj->uid)) ||
+	       ((subj_ids & CRED_EUID) && cred_has_uid(obj, obj_ids, subj->euid));
+}
+
+/**
+ * cred_ids_allow_signal - do its ids allow a subject to signal an object?
+ * @subj: the credentials of whoever sends the signal
+ * @obj:  the credentials of the task it is for
+ *
+ * Return: true if the euid or the uid of @subj is the suid or the uid of @obj.
+ */
+bool cred_ids_allow_signal(const struct cred *subj, const struct cred *obj)
+{
+	return cred_uid_match_any(subj, CRED_EUID | CRED_RUID,
+				  obj, CRED_SUID | CRED_RUID);
+}
+
+/**
+ * cred_ids_allow_sigio - do its ids allow a subject to have SIGIO sent?
+ * @subj: the credentials of whoever made the object the owner of a file
+ * @obj:  the credentials of the task that SIGIO or SIGURG is for
+ *
+ * Return: true if cred_ids_allow_signal() says so, or the euid of @subj is 0.
+ */
+bool cred_ids_allow_sigio(const struct cred *subj, const struct cred *obj)
+{
+	return cred_euid_is_root(subj) || cred_ids_allow_signal(subj, obj);
+}
+
+/**
+ * cred_ids_allow_sched - do its ids allow a subject to schedule an object?
+ * @subj: the credentials of whoever asks
+ * @obj:  the credentials of the task whose policy, affinity or nice value is
+ *        to change
+ *
+ * Return: true if the euid of @subj is the euid or the uid of @obj.
+ */
+bool cred_ids_allow_sched(const struct cred *subj, const struct cred *obj)
+{
+	return cred_uid_match_any(subj, CRED_EUID, obj, CRED_EUID | CRED_RUID);
+}
+
+/**
+ * cred_ids_allow_ioprio - do its ids allow a subject to set an I/O priority?
+ * @subj: the credentials of whoever asks
+ * @obj:  the credentials of the task whose I/O priority is to change
+ *
+ * Return: true if the euid or the uid of @subj is the uid of @obj.
+ */
+bool cred_ids_allow_ioprio(const struct cred *subj, const struct cred *obj)
+{
+	return cred_uid_match_any(subj, CRED_EUID | CRED_RUID, obj, CRED_RUID);
+}
+
+/**
+ * cred_ids_allow_move - do its ids allow a subject to move an object about?
+ * @subj: the credentials of whoever asks
+ * @obj:  the credentials of the task that is to be moved to another group of a
+ *        cgroup v1 hierarchy or of resctrl
+ *
+ * Return: true if the euid of @subj is the uid or the suid of @obj, or is 0.
+ */
+bool cred_ids_allow_move(const struct cred *subj, const struct cred *obj)
+{
+	return cred_euid_is_root(subj) ||
+	       cred_uid_match_any(subj, CRED_EUID, obj, CRED_RUID | CRED_SUID);
+}
+
+/**
+ * cred_ids_allow_control - do its ids give a subject the run of an object?
+ * @subj:  the credentials of whoever asks
+ * @which: which of its ids @subj acts with
+ * @obj:   the credentials of the task that is to be traced, limited or the like
+ *
+ * The strictest of the rules, for what amounts to taking the object over. It
+ * fails for an object that has changed any of its ids, a set-id program say.
+ *
+ * Return: true if the uid @subj acts with is the uid, the euid and the suid of
+ * @obj, and the gid it acts with is the gid, the egid and the sgid of @obj.
+ */
+bool cred_ids_allow_control(const struct cred *subj, enum cred_ids which,
+			    const struct cred *obj)
+{
+	kuid_t uid;
+	kgid_t gid;
+
+	switch (which) {
+	case CRED_IDS_REAL:
+		uid = subj->uid;
+		gid = subj->gid;
+		break;
+	case CRED_IDS_EFFECTIVE:
+		uid = subj->euid;
+		gid = subj->egid;
+		break;
+	case CRED_IDS_FS:
+		uid = subj->fsuid;
+		gid = subj->fsgid;
+		break;
+	default:
+		VFS_WARN_ON_ONCE(1);
+		return false;
+	}
+
+	return uid_eq(uid, obj->uid) && uid_eq(uid, obj->euid) &&
+	       uid_eq(uid, obj->suid) && gid_eq(gid, obj->gid) &&
+	       gid_eq(gid, obj->egid) && gid_eq(gid, obj->sgid);
+}
+
 /**
  * prepare_kernel_cred - Prepare a set of credentials for a kernel service
  * @daemon: A userspace daemon to be used as a reference

-- 
2.55.0

Re: [PATCH v2 01/13] cred: gather the rules for acting on a task by uid

[sashiko-bot]

> A number of operations let one task act on another without any capability,
> because the two belong to the same user: signals, scheduling attributes,
> priorities, resource limits, ptrace and more. Each of them compares the
> ids of the two credentials by hand. There are twelve such places, in six
> flavours that differ in which ids they look at, and nothing says that they
> belong together.
> 
> That makes them hard to review and hard to change. Nobody can tell from
> one of them whether it differs from its neighbour on purpose. Whoever
> wants to know what an equal uid is good for has to find all twelve, and so
> does whoever has to change it. A place that is overlooked is a hole.
> 
> Gather the rules in kernel/cred.c, side by side:
> 
>  rule     of the subject    of the object      euid 0 used for
> [ ... ]
> goes with the operation, remain with the caller.
> 
> The callers are converted one by one in the patches that follow.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-b4-cred-ids-v2-0-956213608f16@toxicpanda.com?part=1

[PATCH v2 02/13] cred: add KUnit tests for the id rules

[Josef Bacik]

Hold each of the new functions against the expression it is about to
replace, word for word as it stands where it is used, for every
combination of a few ids, 0 among them: 729 combinations of three uids
each for subject and object, and 4096 of the twelve ids that
cred_ids_allow_control() can look at.

That the conversions which follow change nothing then does not have to be
taken on trust.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 MAINTAINERS         |   1 +
 kernel/Makefile     |   1 +
 kernel/cred_kunit.c | 218 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/Kconfig.debug   |  13 ++++
 4 files changed, 233 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 93abbb315065..0bc2c39eb417 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7126,6 +7126,7 @@ S:	Supported
 T:	git https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git
 F:	include/linux/cred.h
 F:	kernel/cred.c
+F:	kernel/cred_kunit.c
 F:	rust/kernel/cred.rs
 F:	Documentation/security/credentials.rst
 
diff --git a/kernel/Makefile b/kernel/Makefile
index 4ff92e24680c..da712b4399ff 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -140,6 +140,7 @@ obj-$(CONFIG_RSEQ) += rseq.o
 obj-$(CONFIG_WATCH_QUEUE) += watch_queue.o
 
 obj-$(CONFIG_RESOURCE_KUNIT_TEST) += resource_kunit.o
+obj-$(CONFIG_CRED_KUNIT_TEST) += cred_kunit.o
 obj-$(CONFIG_SYSCTL_KUNIT_TEST) += sysctl-test.o
 obj-$(CONFIG_USER_NS_MAP_KUNIT_TEST) += tests/user_ns_map_kunit.o
 
diff --git a/kernel/cred_kunit.c b/kernel/cred_kunit.c
new file mode 100644
index 000000000000..b3a54e8c565f
--- /dev/null
+++ b/kernel/cred_kunit.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for the rules by which the ids of one credential allow it to act
+ * on another, see "One task acting on another" in kernel/cred.c.
+ *
+ * Each rule is held against the expression it replaced, word for word, as it
+ * stood where it was used, for every combination of a few ids.
+ */
+#include <kunit/test.h>
+#include <linux/cred.h>
+#include <linux/uidgid.h>
+
+/* Root, since two of the rules know it, and two others. */
+static const uid_t ids[] = { 0, 1000, 1001 };
+#define NR_IDS	ARRAY_SIZE(ids)
+
+static void set_uids(struct cred *cred, unsigned int *n)
+{
+	cred->uid = KUIDT_INIT(ids[*n % NR_IDS]);
+	*n /= NR_IDS;
+	cred->euid = KUIDT_INIT(ids[*n % NR_IDS]);
+	*n /= NR_IDS;
+	cred->suid = KUIDT_INIT(ids[*n % NR_IDS]);
+	*n /= NR_IDS;
+}
+
+/* NR_IDS to the power of six: three uids each for the subject and the object. */
+#define NR_UID_COMBINATIONS	(NR_IDS * NR_IDS * NR_IDS * NR_IDS * NR_IDS * NR_IDS)
+
+static void uid_combination(unsigned int n, struct cred *cred, struct cred *tcred)
+{
+	memset(cred, 0, sizeof(*cred));
+	memset(tcred, 0, sizeof(*tcred));
+	set_uids(cred, &n);
+	set_uids(tcred, &n);
+}
+
+/* kill_ok_by_cred() and kill_as_cred_perm() */
+static bool old_signal(const struct cred *cred, const struct cred *tcred)
+{
+	return uid_eq(cred->euid, tcred->suid) ||
+	       uid_eq(cred->euid, tcred->uid) ||
+	       uid_eq(cred->uid, tcred->suid) ||
+	       uid_eq(cred->uid, tcred->uid);
+}
+
+/* sigio_perm(), which had the uid and the euid of the owner in @fown */
+static bool old_sigio(const struct cred *fown, const struct cred *cred)
+{
+	return (uid_eq(fown->euid, GLOBAL_ROOT_UID) ||
+		uid_eq(fown->euid, cred->suid) || uid_eq(fown->euid, cred->uid) ||
+		uid_eq(fown->uid,  cred->suid) || uid_eq(fown->uid,  cred->uid));
+}
+
+/* check_same_owner(), twice */
+static bool old_sched(const struct cred *cred, const struct cred *pcred)
+{
+	return (uid_eq(cred->euid, pcred->euid) ||
+		uid_eq(cred->euid, pcred->uid));
+}
+
+/* set_one_prio_perm() */
+static bool old_nice(const struct cred *cred, const struct cred *pcred)
+{
+	if (uid_eq(pcred->uid,  cred->euid) ||
+	    uid_eq(pcred->euid, cred->euid))
+		return true;
+	return false;
+}
+
+/* set_task_ioprio() */
+static bool old_ioprio(const struct cred *cred, const struct cred *tcred)
+{
+	if (!uid_eq(tcred->uid, cred->euid) &&
+	    !uid_eq(tcred->uid, cred->uid))
+		return false;
+	return true;
+}
+
+/* __cgroup1_procs_write() and rdtgroup_task_write_permission() */
+static bool old_move(const struct cred *cred, const struct cred *tcred)
+{
+	if (!uid_eq(cred->euid, GLOBAL_ROOT_UID) &&
+	    !uid_eq(cred->euid, tcred->uid) &&
+	    !uid_eq(cred->euid, tcred->suid))
+		return false;
+	return true;
+}
+
+static void cred_ids_uid_rules(struct kunit *test)
+{
+	struct cred *cred, *tcred;
+	unsigned int n;
+
+	cred = kunit_kzalloc(test, sizeof(*cred), GFP_KERNEL);
+	tcred = kunit_kzalloc(test, sizeof(*tcred), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, cred);
+	KUNIT_ASSERT_NOT_NULL(test, tcred);
+
+	for (n = 0; n < NR_UID_COMBINATIONS; n++) {
+		uid_combination(n, cred, tcred);
+
+		KUNIT_EXPECT_EQ_MSG(test, cred_ids_allow_signal(cred, tcred),
+				    old_signal(cred, tcred), "combination %u", n);
+		KUNIT_EXPECT_EQ_MSG(test, cred_ids_allow_sigio(cred, tcred),
+				    old_sigio(cred, tcred), "combination %u", n);
+		KUNIT_EXPECT_EQ_MSG(test, cred_ids_allow_sched(cred, tcred),
+				    old_sched(cred, tcred), "combination %u", n);
+		KUNIT_EXPECT_EQ_MSG(test, cred_ids_allow_sched(cred, tcred),
+				    old_nice(cred, tcred), "combination %u", n);
+		KUNIT_EXPECT_EQ_MSG(test, cred_ids_allow_ioprio(cred, tcred),
+				    old_ioprio(cred, tcred), "combination %u", n);
+		KUNIT_EXPECT_EQ_MSG(test, cred_ids_allow_move(cred, tcred),
+				    old_move(cred, tcred), "combination %u", n);
+	}
+}
+
+/* __ptrace_may_access(), with the ids it picked by its mode */
+static bool old_ptrace(kuid_t caller_uid, kgid_t caller_gid,
+		       const struct cred *tcred)
+{
+	if (uid_eq(caller_uid, tcred->euid) &&
+	    uid_eq(caller_uid, tcred->suid) &&
+	    uid_eq(caller_uid, tcred->uid)  &&
+	    gid_eq(caller_gid, tcred->egid) &&
+	    gid_eq(caller_gid, tcred->sgid) &&
+	    gid_eq(caller_gid, tcred->gid))
+		return true;
+	return false;
+}
+
+/* check_prlimit_permission() */
+static bool old_prlimit(const struct cred *cred, const struct cred *tcred)
+{
+	return (uid_eq(cred->uid, tcred->euid) &&
+		uid_eq(cred->uid, tcred->suid) &&
+		uid_eq(cred->uid, tcred->uid)  &&
+		gid_eq(cred->gid, tcred->egid) &&
+		gid_eq(cred->gid, tcred->sgid) &&
+		gid_eq(cred->gid, tcred->gid));
+}
+
+/* keyctl_session_to_parent() */
+static bool old_session_to_parent(const struct cred *mycred,
+				  const struct cred *pcred)
+{
+	if (!uid_eq(pcred->uid,	 mycred->euid) ||
+	    !uid_eq(pcred->euid, mycred->euid) ||
+	    !uid_eq(pcred->suid, mycred->euid) ||
+	    !gid_eq(pcred->gid,	 mycred->egid) ||
+	    !gid_eq(pcred->egid, mycred->egid) ||
+	    !gid_eq(pcred->sgid, mycred->egid))
+		return false;
+	return true;
+}
+
+/*
+ * Two values for each of twelve ids: uid, euid, fsuid and their gids for the
+ * subject, uid, euid, suid and their gids for the object.
+ */
+#define NR_CONTROL_COMBINATIONS	(1U << 12)
+
+static void cred_ids_control_rule(struct kunit *test)
+{
+	struct cred *cred, *tcred;
+	unsigned int n;
+
+	cred = kunit_kzalloc(test, sizeof(*cred), GFP_KERNEL);
+	tcred = kunit_kzalloc(test, sizeof(*tcred), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, cred);
+	KUNIT_ASSERT_NOT_NULL(test, tcred);
+
+	for (n = 0; n < NR_CONTROL_COMBINATIONS; n++) {
+		cred->uid    = KUIDT_INIT(1000 + !!(n & BIT(0)));
+		cred->euid   = KUIDT_INIT(1000 + !!(n & BIT(1)));
+		cred->fsuid  = KUIDT_INIT(1000 + !!(n & BIT(2)));
+		cred->gid    = KGIDT_INIT(1000 + !!(n & BIT(3)));
+		cred->egid   = KGIDT_INIT(1000 + !!(n & BIT(4)));
+		cred->fsgid  = KGIDT_INIT(1000 + !!(n & BIT(5)));
+		tcred->uid   = KUIDT_INIT(1000 + !!(n & BIT(6)));
+		tcred->euid  = KUIDT_INIT(1000 + !!(n & BIT(7)));
+		tcred->suid  = KUIDT_INIT(1000 + !!(n & BIT(8)));
+		tcred->gid   = KGIDT_INIT(1000 + !!(n & BIT(9)));
+		tcred->egid  = KGIDT_INIT(1000 + !!(n & BIT(10)));
+		tcred->sgid  = KGIDT_INIT(1000 + !!(n & BIT(11)));
+
+		KUNIT_EXPECT_EQ_MSG(test,
+				    cred_ids_allow_control(cred, CRED_IDS_FS, tcred),
+				    old_ptrace(cred->fsuid, cred->fsgid, tcred),
+				    "combination %u", n);
+		KUNIT_EXPECT_EQ_MSG(test,
+				    cred_ids_allow_control(cred, CRED_IDS_REAL, tcred),
+				    old_ptrace(cred->uid, cred->gid, tcred),
+				    "combination %u", n);
+		KUNIT_EXPECT_EQ_MSG(test,
+				    cred_ids_allow_control(cred, CRED_IDS_REAL, tcred),
+				    old_prlimit(cred, tcred), "combination %u", n);
+		KUNIT_EXPECT_EQ_MSG(test,
+				    cred_ids_allow_control(cred, CRED_IDS_EFFECTIVE, tcred),
+				    old_session_to_parent(cred, tcred),
+				    "combination %u", n);
+	}
+}
+
+static struct kunit_case cred_ids_cases[] = {
+	KUNIT_CASE(cred_ids_uid_rules),
+	KUNIT_CASE(cred_ids_control_rule),
+	{}
+};
+
+static struct kunit_suite cred_ids_suite = {
+	.name = "cred_ids",
+	.test_cases = cred_ids_cases,
+};
+kunit_test_suite(cred_ids_suite);
+
+MODULE_DESCRIPTION("KUnit tests for the id rules of credentials");
+MODULE_LICENSE("GPL");
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 2d22638ec13a..c548e660dc5c 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2806,6 +2806,19 @@ config HASH_KUNIT_TEST
 	  This is intended to help people writing architecture-specific
 	  optimized versions. If unsure, say N.
 
+config CRED_KUNIT_TEST
+	bool "KUnit test for the id rules of credentials" if !KUNIT_ALL_TESTS
+	depends on KUNIT=y
+	default KUNIT_ALL_TESTS
+	help
+	  This builds the unit test for the rules by which the ids of one
+	  credential allow it to act on another, cred_ids_allow_signal() and
+	  friends. Each rule is held against the expression it replaced.
+	  For more information on KUnit and unit tests in general please refer
+	  to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+	  If unsure, say N.
+
 config RESOURCE_KUNIT_TEST
 	tristate "KUnit test for resource API" if !KUNIT_ALL_TESTS
 	depends on KUNIT

-- 
2.55.0

Re: [PATCH v2 02/13] cred: add KUnit tests for the id rules

[sashiko-bot]

> Hold each of the new functions against the expression it is about to
> replace, word for word as it stands where it is used, for every
> combination of a few ids, 0 among them: 729 combinations of three uids
> each for subject and object, and 4096 of the twelve ids that
> cred_ids_allow_control() can look at.
> 
> That the conversions which follow change nothing then does not have to be
> taken on trust.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-b4-cred-ids-v2-0-956213608f16@toxicpanda.com?part=2

[PATCH v2 03/13] signal: use cred_ids_allow_signal()

[Josef Bacik]

kill_ok_by_cred() and kill_as_cred_perm() have the same four comparisons
each. Use the library.

No functional change.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 kernel/signal.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index dd770e68faee..aba93b99adb6 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -792,10 +792,7 @@ static bool kill_ok_by_cred(struct task_struct *t)
 	const struct cred *cred = current_cred();
 	const struct cred *tcred = __task_cred(t);
 
-	return uid_eq(cred->euid, tcred->suid) ||
-	       uid_eq(cred->euid, tcred->uid) ||
-	       uid_eq(cred->uid, tcred->suid) ||
-	       uid_eq(cred->uid, tcred->uid) ||
+	return cred_ids_allow_signal(cred, tcred) ||
 	       ns_capable(tcred->user_ns, CAP_KILL);
 }
 
@@ -1531,10 +1528,7 @@ static inline bool kill_as_cred_perm(const struct cred *cred,
 {
 	const struct cred *pcred = __task_cred(target);
 
-	return uid_eq(cred->euid, pcred->suid) ||
-	       uid_eq(cred->euid, pcred->uid) ||
-	       uid_eq(cred->uid, pcred->suid) ||
-	       uid_eq(cred->uid, pcred->uid);
+	return cred_ids_allow_signal(cred, pcred);
 }
 
 /*

-- 
2.55.0

Re: [PATCH v2 03/13] signal: use cred_ids_allow_signal()

[sashiko-bot]

> kill_ok_by_cred() and kill_as_cred_perm() have the same four comparisons
> each. Use the library.
> 
> No functional change.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-b4-cred-ids-v2-0-956213608f16@toxicpanda.com?part=3

[PATCH v2 04/13] fcntl: use cred_ids_allow_sigio()

[Josef Bacik]

sigio_perm() has the four comparisons of kill_ok_by_cred(), and lets euid
0 signal anybody. Use the library.

The library compares credentials. F_SETOWN only copies the uid and the
euid of the caller into struct fown_struct, so keep a reference on its
credentials instead, as struct file does for whoever opened it. That makes
the sender of a SIGIO a credential like the sender of any other signal.

F_GETOWNER_UIDS goes on reporting uid 0 for a file whose owner was never
set.

The credentials now stay around until the owner is set again or the file
is released. Clearing the owner leaves them alone, as it left the two ids
alone, so that F_GETOWNER_UIDS goes on reporting them. Who may be
signalled does not change.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/fcntl.c             | 19 ++++++++++---------
 include/linux/fs.h     |  4 ++--
 security/landlock/fs.h |  3 +--
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/fs/fcntl.c b/fs/fcntl.c
index 9b71bc9d71d1..07fe32330bec 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -120,6 +120,7 @@ void file_f_owner_release(struct file *file)
 	f_owner = file_f_owner(file);
 	if (f_owner) {
 		put_pid(f_owner->pid);
+		put_cred(f_owner->cred);
 		kfree(f_owner);
 	}
 }
@@ -140,10 +141,9 @@ void __f_setown(struct file *filp, struct pid *pid, enum pid_type type,
 		f_owner->pid_type = type;
 
 		if (pid) {
-			const struct cred *cred = current_cred();
 			security_file_set_fowner(filp);
-			f_owner->uid = cred->uid;
-			f_owner->euid = cred->euid;
+			put_cred(f_owner->cred);
+			f_owner->cred = get_current_cred();
 		}
 	}
 	write_unlock_irq(&f_owner->lock);
@@ -315,9 +315,12 @@ static int f_getowner_uids(struct file *filp, unsigned long arg)
 
 	f_owner = file_f_owner(filp);
 	if (f_owner) {
+		const struct cred *cred;
+
 		read_lock_irq(&f_owner->lock);
-		src[0] = from_kuid(user_ns, f_owner->uid);
-		src[1] = from_kuid(user_ns, f_owner->euid);
+		cred = f_owner->cred;
+		src[0] = from_kuid(user_ns, cred ? cred->uid : GLOBAL_ROOT_UID);
+		src[1] = from_kuid(user_ns, cred ? cred->euid : GLOBAL_ROOT_UID);
 		read_unlock_irq(&f_owner->lock);
 	}
 
@@ -847,10 +850,8 @@ static inline int sigio_perm(struct task_struct *p,
 
 	rcu_read_lock();
 	cred = __task_cred(p);
-	ret = ((uid_eq(fown->euid, GLOBAL_ROOT_UID) ||
-		uid_eq(fown->euid, cred->suid) || uid_eq(fown->euid, cred->uid) ||
-		uid_eq(fown->uid,  cred->suid) || uid_eq(fown->uid,  cred->uid)) &&
-	       !security_file_send_sigiotask(p, fown, sig));
+	ret = cred_ids_allow_sigio(fown->cred, cred) &&
+	      !security_file_send_sigiotask(p, fown, sig);
 	rcu_read_unlock();
 	return ret;
 }
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 99caef43ff3c..88748be03505 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1185,10 +1185,10 @@ static inline unsigned imajor(const struct inode *inode)
 
 struct fown_struct {
 	struct file *file;	/* backpointer for security modules */
-	rwlock_t lock;          /* protects pid, uid, euid fields */
+	rwlock_t lock;          /* protects pid, cred fields */
 	struct pid *pid;	/* pid or -pgrp where SIGIO should be sent */
 	enum pid_type pid_type;	/* Kind of process group SIGIO should be sent to */
-	kuid_t uid, euid;	/* uid/euid of process setting the owner */
+	const struct cred *cred; /* of the process setting the owner */
 	int signum;		/* posix.1b rt signal to be delivered on IO */
 };
 
diff --git a/security/landlock/fs.h b/security/landlock/fs.h
index 50b3ddb8ac1c..d3aa84820594 100644
--- a/security/landlock/fs.h
+++ b/security/landlock/fs.h
@@ -84,8 +84,7 @@ struct landlock_file_security {
 	 * @fown_subject: Landlock credential of the task that set the PID that
 	 * may receive a signal e.g., SIGURG when writing MSG_OOB to the
 	 * related socket.  This pointer is protected by the related
-	 * file->f_owner->lock, as for fown_struct's members: pid, uid, and
-	 * euid.
+	 * file->f_owner->lock, as for fown_struct's members: pid and cred.
 	 */
 	struct landlock_cred_security fown_subject;
 	/**

-- 
2.55.0

Re: [PATCH v2 04/13] fcntl: use cred_ids_allow_sigio()

[sashiko-bot]

> sigio_perm() has the four comparisons of kill_ok_by_cred(), and lets euid
> 0 signal anybody. Use the library.
> 
> The library compares credentials. F_SETOWN only copies the uid and the
> euid of the caller into struct fown_struct, so keep a reference on its
> credentials instead, as struct file does for whoever opened it. That makes
> the sender of a SIGIO a credential like the sender of any other signal.
> 
> F_GETOWNER_UIDS goes on reporting uid 0 for a file whose owner was never
> set.
> 
> The credentials now stay around until the owner is set again or the file
> is released. Clearing the owner leaves them alone, as it left the two ids
> alone, so that F_GETOWNER_UIDS goes on reporting them. Who may be
> signalled does not change.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-b4-cred-ids-v2-0-956213608f16@toxicpanda.com?part=4

[PATCH v2 05/13] sched: use cred_ids_allow_sched()

[Josef Bacik]

Use the library in check_same_owner().

No functional change.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 kernel/sched/syscalls.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
index 45a127deeadd..31650000d6cc 100644
--- a/kernel/sched/syscalls.c
+++ b/kernel/sched/syscalls.c
@@ -280,8 +280,7 @@ static bool check_same_owner(struct task_struct *p)
 	guard(rcu)();
 
 	pcred = __task_cred(p);
-	return (uid_eq(cred->euid, pcred->euid) ||
-		uid_eq(cred->euid, pcred->uid));
+	return cred_ids_allow_sched(cred, pcred);
 }
 
 #ifdef CONFIG_RT_MUTEXES

-- 
2.55.0

Re: [PATCH v2 05/13] sched: use cred_ids_allow_sched()

[sashiko-bot]

> Use the library in check_same_owner().
> 
> No functional change.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-b4-cred-ids-v2-0-956213608f16@toxicpanda.com?part=5

[PATCH v2 06/13] MIPS: use cred_ids_allow_sched()

[Josef Bacik]

The FPU affinity code has a copy of check_same_owner() from the scheduler.
Use the library there as well.

No functional change. Compile tested only.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 arch/mips/kernel/mips-mt-fpaff.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/mips/kernel/mips-mt-fpaff.c b/arch/mips/kernel/mips-mt-fpaff.c
index 4fead87d2f43..1ac86a897956 100644
--- a/arch/mips/kernel/mips-mt-fpaff.c
+++ b/arch/mips/kernel/mips-mt-fpaff.c
@@ -54,8 +54,7 @@ static bool check_same_owner(struct task_struct *p)
 
 	rcu_read_lock();
 	pcred = __task_cred(p);
-	match = (uid_eq(cred->euid, pcred->euid) ||
-		 uid_eq(cred->euid, pcred->uid));
+	match = cred_ids_allow_sched(cred, pcred);
 	rcu_read_unlock();
 	return match;
 }

-- 
2.55.0

Re: [PATCH v2 06/13] MIPS: use cred_ids_allow_sched()

[sashiko-bot]

> The FPU affinity code has a copy of check_same_owner() from the scheduler.
> Use the library there as well.
> 
> No functional change. Compile tested only.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-b4-cred-ids-v2-0-956213608f16@toxicpanda.com?part=6

[PATCH v2 07/13] sys: use cred_ids_allow_sched() for setpriority()

[Josef Bacik]

set_one_prio_perm() makes the two comparisons of check_same_owner(),
written the other way around. Use the library.

No functional change.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 kernel/sys.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/sys.c b/kernel/sys.c
index 755bfa5ccd92..e43491a9dceb 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -220,8 +220,7 @@ static bool set_one_prio_perm(struct task_struct *p)
 {
 	const struct cred *cred = current_cred(), *pcred = __task_cred(p);
 
-	if (uid_eq(pcred->uid,  cred->euid) ||
-	    uid_eq(pcred->euid, cred->euid))
+	if (cred_ids_allow_sched(cred, pcred))
 		return true;
 	if (ns_capable(pcred->user_ns, CAP_SYS_NICE))
 		return true;

-- 
2.55.0

Re: [PATCH v2 07/13] sys: use cred_ids_allow_sched() for setpriority()

[sashiko-bot]

> set_one_prio_perm() makes the two comparisons of check_same_owner(),
> written the other way around. Use the library.
> 
> No functional change.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-b4-cred-ids-v2-0-956213608f16@toxicpanda.com?part=7

[PATCH v2 08/13] sys: use cred_ids_allow_control() for prlimit()

[Josef Bacik]

check_prlimit_permission() wants the real uid and gid of the caller to be
all three of the uids and gids of the target. Use the library.

No functional change.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 kernel/sys.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/kernel/sys.c b/kernel/sys.c
index e43491a9dceb..b341910f24ee 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1716,12 +1716,7 @@ static int check_prlimit_permission(struct task_struct *task,
 		return 0;
 
 	tcred = __task_cred(task);
-	id_match = (uid_eq(cred->uid, tcred->euid) &&
-		    uid_eq(cred->uid, tcred->suid) &&
-		    uid_eq(cred->uid, tcred->uid)  &&
-		    gid_eq(cred->gid, tcred->egid) &&
-		    gid_eq(cred->gid, tcred->sgid) &&
-		    gid_eq(cred->gid, tcred->gid));
+	id_match = cred_ids_allow_control(cred, CRED_IDS_REAL, tcred);
 	if (!id_match && !ns_capable(tcred->user_ns, CAP_SYS_RESOURCE))
 		return -EPERM;
 

-- 
2.55.0

Re: [PATCH v2 08/13] sys: use cred_ids_allow_control() for prlimit()

[sashiko-bot]

> check_prlimit_permission() wants the real uid and gid of the caller to be
> all three of the uids and gids of the target. Use the library.
> 
> No functional change.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-b4-cred-ids-v2-0-956213608f16@toxicpanda.com?part=8

[PATCH v2 09/13] block: use cred_ids_allow_ioprio()

[Josef Bacik]

Use the library in set_task_ioprio().

No functional change.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 block/blk-ioc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index d15918d7fabb..80e1ed2db8c8 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -248,8 +248,7 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
 
 	rcu_read_lock();
 	tcred = __task_cred(task);
-	if (!uid_eq(tcred->uid, cred->euid) &&
-	    !uid_eq(tcred->uid, cred->uid) && !capable(CAP_SYS_NICE)) {
+	if (!cred_ids_allow_ioprio(cred, tcred) && !capable(CAP_SYS_NICE)) {
 		rcu_read_unlock();
 		return -EPERM;
 	}

-- 
2.55.0

Re: [PATCH v2 09/13] block: use cred_ids_allow_ioprio()

[sashiko-bot]

> Use the library in set_task_ioprio().
> 
> No functional change.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-b4-cred-ids-v2-0-956213608f16@toxicpanda.com?part=9

[PATCH v2 10/13] cgroup/v1: use cred_ids_allow_move()

[Josef Bacik]

Use the library in __cgroup1_procs_write(). That euid 0 may move anybody
is part of the rule.

No functional change.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 kernel/cgroup/cgroup-v1.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
index 7f55a7bc8402..216a2dcdde86 100644
--- a/kernel/cgroup/cgroup-v1.c
+++ b/kernel/cgroup/cgroup-v1.c
@@ -521,9 +521,7 @@ static ssize_t __cgroup1_procs_write(struct kernfs_open_file *of,
 	 */
 	cred = of->file->f_cred;
 	tcred = get_task_cred(task);
-	if (!uid_eq(cred->euid, GLOBAL_ROOT_UID) &&
-	    !uid_eq(cred->euid, tcred->uid) &&
-	    !uid_eq(cred->euid, tcred->suid))
+	if (!cred_ids_allow_move(cred, tcred))
 		ret = -EACCES;
 	put_cred(tcred);
 	if (ret)

-- 
2.55.0

Re: [PATCH v2 10/13] cgroup/v1: use cred_ids_allow_move()

[sashiko-bot]

> Use the library in __cgroup1_procs_write(). That euid 0 may move anybody
> is part of the rule.
> 
> No functional change.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-b4-cred-ids-v2-0-956213608f16@toxicpanda.com?part=10

[PATCH v2 11/13] fs/resctrl: use cred_ids_allow_move()

[Josef Bacik]

rdtgroup_task_write_permission() makes the comparisons of the cgroup v1
code, euid 0 included. Use the library.

No functional change.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/resctrl/rdtgroup.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 57ab090072c2..6b7b7daa9fd3 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -732,9 +732,7 @@ static int rdtgroup_task_write_permission(struct task_struct *task,
 	 * Even if we're attaching all tasks in the thread group, we only
 	 * need to check permissions on one of them.
 	 */
-	if (!uid_eq(cred->euid, GLOBAL_ROOT_UID) &&
-	    !uid_eq(cred->euid, tcred->uid) &&
-	    !uid_eq(cred->euid, tcred->suid)) {
+	if (!cred_ids_allow_move(cred, tcred)) {
 		rdt_last_cmd_printf("No permission to move task %d\n", task->pid);
 		ret = -EPERM;
 	}

-- 
2.55.0

Re: [PATCH v2 11/13] fs/resctrl: use cred_ids_allow_move()

[sashiko-bot]

> rdtgroup_task_write_permission() makes the comparisons of the cgroup v1
> code, euid 0 included. Use the library.
> 
> No functional change.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-b4-cred-ids-v2-0-956213608f16@toxicpanda.com?part=11

[PATCH v2 12/13] ptrace: use cred_ids_allow_control()

[Josef Bacik]

__ptrace_may_access() picks the filesystem or the real uid and gid of the
caller by its mode and wants them to be all three of the uids and gids of
the target. Say which ids it is and use the library.

No functional change.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 kernel/ptrace.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 3f2fe68ccb14..8024d80f82e3 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -305,8 +305,7 @@ static bool task_still_dumpable(struct task_struct *task, unsigned int mode)
 static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
 {
 	const struct cred *cred = current_cred(), *tcred;
-	kuid_t caller_uid;
-	kgid_t caller_gid;
+	enum cred_ids caller_ids;
 
 	if (!(mode & PTRACE_MODE_FSCREDS) == !(mode & PTRACE_MODE_REALCREDS)) {
 		WARN(1, "denying ptrace access check without PTRACE_MODE_*CREDS\n");
@@ -327,8 +326,7 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
 		return 0;
 	rcu_read_lock();
 	if (mode & PTRACE_MODE_FSCREDS) {
-		caller_uid = cred->fsuid;
-		caller_gid = cred->fsgid;
+		caller_ids = CRED_IDS_FS;
 	} else {
 		/*
 		 * Using the euid would make more sense here, but something
@@ -338,16 +336,10 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
 		 * used a syscall that requests access to another process
 		 * (and not a filesystem syscall to procfs).
 		 */
-		caller_uid = cred->uid;
-		caller_gid = cred->gid;
+		caller_ids = CRED_IDS_REAL;
 	}
 	tcred = __task_cred(task);
-	if (uid_eq(caller_uid, tcred->euid) &&
-	    uid_eq(caller_uid, tcred->suid) &&
-	    uid_eq(caller_uid, tcred->uid)  &&
-	    gid_eq(caller_gid, tcred->egid) &&
-	    gid_eq(caller_gid, tcred->sgid) &&
-	    gid_eq(caller_gid, tcred->gid))
+	if (cred_ids_allow_control(cred, caller_ids, tcred))
 		goto ok;
 	if (ptrace_has_cap(tcred->user_ns, mode))
 		goto ok;

-- 
2.55.0

Re: [PATCH v2 12/13] ptrace: use cred_ids_allow_control()

[sashiko-bot]

> __ptrace_may_access() picks the filesystem or the real uid and gid of the
> caller by its mode and wants them to be all three of the uids and gids of
> the target. Say which ids it is and use the library.
> 
> No functional change.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-b4-cred-ids-v2-0-956213608f16@toxicpanda.com?part=12

[PATCH v2 13/13] keys: use cred_ids_allow_control()

[Josef Bacik]

KEYCTL_SESSION_TO_PARENT wants the effective uid and gid of the caller to
be all three of the uids and gids of its parent. Use the library.

No functional change.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 security/keys/keyctl.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 36ffba0ebba6..cf963251a87a 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1692,12 +1692,7 @@ long keyctl_session_to_parent(void)
 
 	/* the parent must have the same effective ownership and mustn't be
 	 * SUID/SGID */
-	if (!uid_eq(pcred->uid,	 mycred->euid) ||
-	    !uid_eq(pcred->euid, mycred->euid) ||
-	    !uid_eq(pcred->suid, mycred->euid) ||
-	    !gid_eq(pcred->gid,	 mycred->egid) ||
-	    !gid_eq(pcred->egid, mycred->egid) ||
-	    !gid_eq(pcred->sgid, mycred->egid))
+	if (!cred_ids_allow_control(mycred, CRED_IDS_EFFECTIVE, pcred))
 		goto unlock;
 
 	/* the keyrings must have the same UID */

-- 
2.55.0

Re: [PATCH v2 13/13] keys: use cred_ids_allow_control()

[sashiko-bot]

> KEYCTL_SESSION_TO_PARENT wants the effective uid and gid of the caller to
> be all three of the uids and gids of its parent. Use the library.
> 
> No functional change.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-b4-cred-ids-v2-0-956213608f16@toxicpanda.com?part=13