A cell with a single id runs with the kuid of the user who created it. That causes problems in both directions: - The pid namespace prevents the cell from naming tasks outside of it. But the pids of the cell are visible from above. So every other task of that user passes the checks that compare uids, and can signal, renice or limit the tasks of the cell. The authority over a cell is supposed to be the cellfd. - The parent of a task that was spawned into a cell is the spawner, which is outside of the cell. With KEYCTL_SESSION_TO_PARENT the task could replace the session keyring of the spawner with one that it controls. The ids of two credentials can only be compared if both belong to the same cell. All the rules that let one task act on another because of its ids are now in one place, so the restriction can be added once. Add cred_ids_comparable(), and make every rule call it first. That covers signals and SIGIO, scheduling attributes and nice values, I/O priorities, resource limits, ptrace, moving a task in a cgroup v1 hierarchy or between resctrl groups, and KEYCTL_SESSION_TO_PARENT. It also applies to euid 0. Some of these rules let a task with euid 0 send SIGIO to any task, or move any task in a cgroup v1 hierarchy or between resctrl groups. A cell is a hard boundary for root as well. A cell that misbehaves can be killed and built again. The capability checks that sit next to most of these rules are not changed. A capability that is held in an ancestor user namespace still applies to a task of a cell for now. Signed-off-by: Josef Bacik --- kernel/cred.c | 28 ++++++++++++++++++++++++++++ kernel/cred_kunit.c | 5 +++++ 2 files changed, 33 insertions(+) diff --git a/kernel/cred.c b/kernel/cred.c index 3aa1b1e3eade..72df625f8c99 100644 --- a/kernel/cred.c +++ b/kernel/cred.c @@ -577,6 +577,10 @@ void __init cred_init(void) * 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. * + * It is also the one place to say when the ids of two credentials are not to be + * compared at all, in which case every rule says no. They are not across cells, + * see Documentation/userspace-api/cells.rst. + * * 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. * @@ -588,6 +592,18 @@ void __init cred_init(void) #define CRED_EUID BIT(1) #define CRED_SUID BIT(2) +/* + * Are the ids of two credentials to be compared at all? Every rule below asks + * this first. + * + * Across cells an id means nothing, 0 included. A one-id cell runs on the ids + * of whoever made it, and authority over a cell is a handle. + */ +static bool cred_ids_comparable(const struct cred *subj, const struct cred *obj) +{ + return cell_same(subj, obj); +} + static bool cred_euid_is_root(const struct cred *subj) { return uid_eq(subj->euid, GLOBAL_ROOT_UID); @@ -607,6 +623,9 @@ static bool cred_has_uid(const struct cred *cred, unsigned int ids, kuid_t uid) static bool cred_uid_match_any(const struct cred *subj, unsigned int subj_ids, const struct cred *obj, unsigned int obj_ids) { + if (!cred_ids_comparable(subj, obj)) + return false; + 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)); } @@ -633,6 +652,9 @@ 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) { + if (!cred_ids_comparable(subj, obj)) + return false; + return cred_euid_is_root(subj) || cred_ids_allow_signal(subj, obj); } @@ -671,6 +693,9 @@ 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) { + if (!cred_ids_comparable(subj, obj)) + return false; + return cred_euid_is_root(subj) || cred_uid_match_any(subj, CRED_EUID, obj, CRED_RUID | CRED_SUID); } @@ -693,6 +718,9 @@ bool cred_ids_allow_control(const struct cred *subj, enum cred_ids which, kuid_t uid; kgid_t gid; + if (!cred_ids_comparable(subj, obj)) + return false; + switch (which) { case CRED_IDS_REAL: uid = subj->uid; diff --git a/kernel/cred_kunit.c b/kernel/cred_kunit.c index b3a54e8c565f..3921e8f2716f 100644 --- a/kernel/cred_kunit.c +++ b/kernel/cred_kunit.c @@ -9,6 +9,7 @@ #include #include #include +#include /* Root, since two of the rules know it, and two others. */ static const uid_t ids[] = { 0, 1000, 1001 }; @@ -31,6 +32,8 @@ static void uid_combination(unsigned int n, struct cred *cred, struct cred *tcre { memset(cred, 0, sizeof(*cred)); memset(tcred, 0, sizeof(*tcred)); + cred->user_ns = &init_user_ns; + tcred->user_ns = &init_user_ns; set_uids(cred, &n); set_uids(tcred, &n); } @@ -169,6 +172,8 @@ static void cred_ids_control_rule(struct kunit *test) tcred = kunit_kzalloc(test, sizeof(*tcred), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, cred); KUNIT_ASSERT_NOT_NULL(test, tcred); + cred->user_ns = &init_user_ns; + tcred->user_ns = &init_user_ns; for (n = 0; n < NR_CONTROL_COMBINATIONS; n++) { cred->uid = KUIDT_INIT(1000 + !!(n & BIT(0))); -- 2.55.0