[PATCH RFC] security/KEYS: get rid of cred_alloc_blank and cred_transfer
Jarkko Sakkinen
jarkko.sakkinen at iki.fi
Fri Aug 2 18:09:04 UTC 2024
On Fri Aug 2, 2024 at 4:10 PM EEST, Jann Horn wrote:
> cred_alloc_blank and cred_transfer were only necessary so that keyctl can
> allocate creds in the child and then asynchronously have the parent fill
> them in and apply them.
>
> Get rid of them by letting the child synchronously wait for the task work
> executing in the parent's context. This way, any errors that happen in the
> task work can be plumbed back into the syscall return value in the child.
>
> Note that this requires using TWA_SIGNAL instead of TWA_RESUME, so the
> parent might observe some spurious -EGAIN syscall returns or such; but the
> parent likely anyway has to be ready to deal with the side effects of
> receiving signals (since it'll probably get SIGCHLD when the child dies),
> so that probably isn't an issue.
>
> Signed-off-by: Jann Horn <jannh at google.com>
> ---
> This is a quickly hacked up demo of the approach I proposed at
> <https://lore.kernel.org/all/CAG48ez2bnvuX8i-D=5DxmfzEOKTWAf-DkgQq6aNC4WzSGoEGHg@mail.gmail.com/>
> to get rid of the cred_transfer stuff. Diffstat looks like this:
>
> include/linux/cred.h | 1 -
> include/linux/lsm_hook_defs.h | 3 ---
> include/linux/security.h | 12 ------------
> kernel/cred.c | 23 -----------------------
> security/apparmor/lsm.c | 19 -------------------
> security/keys/internal.h | 8 ++++++++
> security/keys/keyctl.c | 100 ++++++++++++++++++++++++++--------------------------------------------------------------------------
> security/keys/process_keys.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------
> security/landlock/cred.c | 11 ++---------
> security/security.c | 35 -----------------------------------
> security/selinux/hooks.c | 12 ------------
> security/smack/smack_lsm.c | 32 --------------------------------
> 12 files changed, 82 insertions(+), 260 deletions(-)
>
> What do you think? Synchronously waiting for task work is a bit ugly,
> but at least this condenses the uglyness in the keys subsystem instead
> of making the rest of the security subsystem deal with this stuff.
Why does synchronously waiting is ugly? Not sarcasm, I genuineily
interested of breaking that down in smaller pieces.
E.g. what disadvantages would be there from your point of view?
Only trying to form a common picture, that's all.
>
> Another approach to simplify things further would be to try to move
> the session keyring out of the creds entirely and just let the child
> update it directly with appropriate locking, but I don't know enough
> about the keys subsystem to know if that would maybe break stuff
> that relies on override_creds() also overriding the keyrings, or
> something like that.
> ---
> include/linux/cred.h | 1 -
> include/linux/lsm_hook_defs.h | 3 --
> include/linux/security.h | 12 -----
> kernel/cred.c | 23 ----------
> security/apparmor/lsm.c | 19 --------
> security/keys/internal.h | 8 ++++
> security/keys/keyctl.c | 100 +++++++++++-------------------------------
> security/keys/process_keys.c | 86 +++++++++++++++++++-----------------
> security/landlock/cred.c | 11 +----
> security/security.c | 35 ---------------
> security/selinux/hooks.c | 12 -----
> security/smack/smack_lsm.c | 32 --------------
> 12 files changed, 82 insertions(+), 260 deletions(-)
Given the large patch size:
1. If it is impossible to split some meaningful patches, i.e. patches
that transform kernel tree from working state to another, I can
cope with this.
2. Even for small chunks that can be split into their own logical
pieces: please do that. Helps to review the main gist later on.
>
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index 2976f534a7a3..54b5105d4cd5 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -147,13 +147,12 @@ struct cred {
> } __randomize_layout;
>
> extern void __put_cred(struct cred *);
> extern void exit_creds(struct task_struct *);
> extern int copy_creds(struct task_struct *, unsigned long);
> extern const struct cred *get_task_cred(struct task_struct *);
> -extern struct cred *cred_alloc_blank(void);
> extern struct cred *prepare_creds(void);
> extern struct cred *prepare_exec_creds(void);
> extern int commit_creds(struct cred *);
> extern void abort_creds(struct cred *);
> extern const struct cred *override_creds(const struct cred *);
> extern void revert_creds(const struct cred *);
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 855db460e08b..1d75075cb607 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -204,18 +204,15 @@ LSM_HOOK(int, 0, file_receive, struct file *file)
> LSM_HOOK(int, 0, file_open, struct file *file)
> LSM_HOOK(int, 0, file_post_open, struct file *file, int mask)
> LSM_HOOK(int, 0, file_truncate, struct file *file)
> LSM_HOOK(int, 0, task_alloc, struct task_struct *task,
> unsigned long clone_flags)
> LSM_HOOK(void, LSM_RET_VOID, task_free, struct task_struct *task)
> -LSM_HOOK(int, 0, cred_alloc_blank, struct cred *cred, gfp_t gfp)
> LSM_HOOK(void, LSM_RET_VOID, cred_free, struct cred *cred)
> LSM_HOOK(int, 0, cred_prepare, struct cred *new, const struct cred *old,
> gfp_t gfp)
> -LSM_HOOK(void, LSM_RET_VOID, cred_transfer, struct cred *new,
> - const struct cred *old)
> LSM_HOOK(void, LSM_RET_VOID, cred_getsecid, const struct cred *c, u32 *secid)
> LSM_HOOK(int, 0, kernel_act_as, struct cred *new, u32 secid)
> LSM_HOOK(int, 0, kernel_create_files_as, struct cred *new, struct inode *inode)
> LSM_HOOK(int, 0, kernel_module_request, char *kmod_name)
> LSM_HOOK(int, 0, kernel_load_data, enum kernel_load_data_id id, bool contents)
> LSM_HOOK(int, 0, kernel_post_load_data, char *buf, loff_t size,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 1390f1efb4f0..a366c2a03f55 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -421,16 +421,14 @@ int security_file_send_sigiotask(struct task_struct *tsk,
> int security_file_receive(struct file *file);
> int security_file_open(struct file *file);
> int security_file_post_open(struct file *file, int mask);
> int security_file_truncate(struct file *file);
> int security_task_alloc(struct task_struct *task, unsigned long clone_flags);
> void security_task_free(struct task_struct *task);
> -int security_cred_alloc_blank(struct cred *cred, gfp_t gfp);
> void security_cred_free(struct cred *cred);
> int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp);
> -void security_transfer_creds(struct cred *new, const struct cred *old);
> void security_cred_getsecid(const struct cred *c, u32 *secid);
> int security_kernel_act_as(struct cred *new, u32 secid);
> int security_kernel_create_files_as(struct cred *new, struct inode *inode);
> int security_kernel_module_request(char *kmod_name);
> int security_kernel_load_data(enum kernel_load_data_id id, bool contents);
> int security_kernel_post_load_data(char *buf, loff_t size,
> @@ -1117,32 +1115,22 @@ static inline int security_task_alloc(struct task_struct *task,
> return 0;
> }
>
> static inline void security_task_free(struct task_struct *task)
> { }
>
> -static inline int security_cred_alloc_blank(struct cred *cred, gfp_t gfp)
> -{
> - return 0;
> -}
> -
> static inline void security_cred_free(struct cred *cred)
> { }
>
> static inline int security_prepare_creds(struct cred *new,
> const struct cred *old,
> gfp_t gfp)
> {
> return 0;
> }
>
> -static inline void security_transfer_creds(struct cred *new,
> - const struct cred *old)
> -{
> -}
> -
> static inline void security_cred_getsecid(const struct cred *c, u32 *secid)
> {
> *secid = 0;
> }
>
> static inline int security_kernel_act_as(struct cred *cred, u32 secid)
> diff --git a/kernel/cred.c b/kernel/cred.c
> index 075cfa7c896f..b2f6130cd6b7 100644
> --- a/kernel/cred.c
> +++ b/kernel/cred.c
> @@ -163,35 +163,12 @@ const struct cred *get_task_cred(struct task_struct *task)
>
> rcu_read_unlock();
> return cred;
> }
> EXPORT_SYMBOL(get_task_cred);
>
> -/*
> - * Allocate blank credentials, such that the credentials can be filled in at a
> - * later date without risk of ENOMEM.
> - */
> -struct cred *cred_alloc_blank(void)
> -{
> - struct cred *new;
> -
> - new = kmem_cache_zalloc(cred_jar, GFP_KERNEL);
> - if (!new)
> - return NULL;
> -
> - atomic_long_set(&new->usage, 1);
> - if (security_cred_alloc_blank(new, GFP_KERNEL_ACCOUNT) < 0)
> - goto error;
> -
> - return new;
> -
> -error:
> - abort_creds(new);
> - return NULL;
> -}
> -
> /**
> * prepare_creds - Prepare a new set of credentials for modification
> *
> * Prepare a new set of task credentials for modification. A task's creds
> * shouldn't generally be modified directly, therefore this function is used to
> * prepare a new copy, which the caller then modifies and then commits by
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 808060f9effb..089d53978d9e 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -74,39 +74,22 @@ static DEFINE_PER_CPU(struct aa_local_cache, aa_local_buffers);
> static void apparmor_cred_free(struct cred *cred)
> {
> aa_put_label(cred_label(cred));
> set_cred_label(cred, NULL);
> }
>
> -/*
> - * allocate the apparmor part of blank credentials
> - */
> -static int apparmor_cred_alloc_blank(struct cred *cred, gfp_t gfp)
> -{
> - set_cred_label(cred, NULL);
> - return 0;
> -}
> -
> /*
> * prepare new cred label for modification by prepare_cred block
> */
> static int apparmor_cred_prepare(struct cred *new, const struct cred *old,
> gfp_t gfp)
> {
> set_cred_label(new, aa_get_newest_label(cred_label(old)));
> return 0;
> }
>
> -/*
> - * transfer the apparmor data to a blank set of creds
> - */
> -static void apparmor_cred_transfer(struct cred *new, const struct cred *old)
> -{
> - set_cred_label(new, aa_get_newest_label(cred_label(old)));
> -}
> -
> static void apparmor_task_free(struct task_struct *task)
> {
>
> aa_free_task_ctx(task_ctx(task));
> }
>
> @@ -1504,16 +1487,14 @@ static struct security_hook_list apparmor_hooks[] __ro_after_init = {
> apparmor_socket_getpeersec_dgram),
> LSM_HOOK_INIT(sock_graft, apparmor_sock_graft),
> #ifdef CONFIG_NETWORK_SECMARK
> LSM_HOOK_INIT(inet_conn_request, apparmor_inet_conn_request),
> #endif
>
> - LSM_HOOK_INIT(cred_alloc_blank, apparmor_cred_alloc_blank),
> LSM_HOOK_INIT(cred_free, apparmor_cred_free),
> LSM_HOOK_INIT(cred_prepare, apparmor_cred_prepare),
> - LSM_HOOK_INIT(cred_transfer, apparmor_cred_transfer),
>
> LSM_HOOK_INIT(bprm_creds_for_exec, apparmor_bprm_creds_for_exec),
> LSM_HOOK_INIT(bprm_committing_creds, apparmor_bprm_committing_creds),
> LSM_HOOK_INIT(bprm_committed_creds, apparmor_bprm_committed_creds),
>
> LSM_HOOK_INIT(task_free, apparmor_task_free),
> diff --git a/security/keys/internal.h b/security/keys/internal.h
> index 2cffa6dc8255..2c5eadc04cf2 100644
> --- a/security/keys/internal.h
> +++ b/security/keys/internal.h
> @@ -157,12 +157,20 @@ extern struct key *request_key_and_link(struct key_type *type,
> unsigned long flags);
>
> extern bool lookup_user_key_possessed(const struct key *key,
> const struct key_match_data *match_data);
>
> extern long join_session_keyring(const char *name);
> +
> +struct keyctl_session_to_parent_context {
> + struct callback_head work;
> + struct completion done;
> + struct key *new_session_keyring;
> + const struct cred *child_cred;
> + int result;
> +};
> extern void key_change_session_keyring(struct callback_head *twork);
>
> extern struct work_struct key_gc_work;
> extern unsigned key_gc_delay;
> extern void keyring_gc(struct key *keyring, time64_t limit);
> extern void keyring_restriction_gc(struct key *keyring,
> diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> index ab927a142f51..116ef2dfa5a1 100644
> --- a/security/keys/keyctl.c
> +++ b/security/keys/keyctl.c
> @@ -1616,104 +1616,56 @@ long keyctl_get_security(key_serial_t keyid,
> * parent process.
> *
> * The keyring must exist and must grant the caller LINK permission, and the
> * parent process must be single-threaded and must have the same effective
> * ownership as this process and mustn't be SUID/SGID.
> *
> - * The keyring will be emplaced on the parent when it next resumes userspace.
> + * The keyring will be emplaced on the parent via a pseudo-signal.
> *
> * If successful, 0 will be returned.
> */
> long keyctl_session_to_parent(void)
> {
> - struct task_struct *me, *parent;
> - const struct cred *mycred, *pcred;
> - struct callback_head *newwork, *oldwork;
> + struct keyctl_session_to_parent_context ctx;
> + struct task_struct *parent;
> key_ref_t keyring_r;
> - struct cred *cred;
> int ret;
>
> keyring_r = lookup_user_key(KEY_SPEC_SESSION_KEYRING, 0, KEY_NEED_LINK);
> if (IS_ERR(keyring_r))
> return PTR_ERR(keyring_r);
>
> - ret = -ENOMEM;
> -
> - /* our parent is going to need a new cred struct, a new tgcred struct
> - * and new security data, so we allocate them here to prevent ENOMEM in
> - * our parent */
> - cred = cred_alloc_blank();
> - if (!cred)
> - goto error_keyring;
> - newwork = &cred->rcu;
> + write_lock_irq(&tasklist_lock);
> + parent = get_task_struct(rcu_dereference_protected(current->real_parent,
> + lockdep_is_held(&tasklist_lock)));
> + write_unlock_irq(&tasklist_lock);
>
> - cred->session_keyring = key_ref_to_ptr(keyring_r);
> - keyring_r = NULL;
> - init_task_work(newwork, key_change_session_keyring);
> + /* the parent mustn't be init and mustn't be a kernel thread */
> + if (is_global_init(parent) || (READ_ONCE(parent->flags) & PF_KTHREAD) != 0)
> + goto put_task;
>
> - me = current;
> - rcu_read_lock();
> - write_lock_irq(&tasklist_lock);
> + ctx.new_session_keyring = key_ref_to_ptr(keyring_r);
> + ctx.child_cred = current_cred();
> + init_completion(&ctx.done);
> + init_task_work(&ctx.work, key_change_session_keyring);
> + ret = task_work_add(parent, &ctx.work, TWA_SIGNAL);
> + if (ret)
> + goto put_task;
>
> - ret = -EPERM;
> - oldwork = NULL;
> - parent = rcu_dereference_protected(me->real_parent,
> - lockdep_is_held(&tasklist_lock));
> + ret = wait_for_completion_killable(&ctx.done);
>
> - /* the parent mustn't be init and mustn't be a kernel thread */
> - if (parent->pid <= 1 || !parent->mm)
> - goto unlock;
> -
> - /* the parent must be single threaded */
> - if (!thread_group_empty(parent))
> - goto unlock;
> -
> - /* the parent and the child must have different session keyrings or
> - * there's no point */
> - mycred = current_cred();
> - pcred = __task_cred(parent);
> - if (mycred == pcred ||
> - mycred->session_keyring == pcred->session_keyring) {
> - ret = 0;
> - goto unlock;
> + if (task_work_cancel(parent, &ctx.work)) {
> + /* we got killed, task work has been cancelled */
> + } else {
> + /* task work is running or has been executed */
> + wait_for_completion(&ctx.done);
> + ret = ctx.result;
> }
>
> - /* 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))
> - goto unlock;
> -
> - /* the keyrings must have the same UID */
> - if ((pcred->session_keyring &&
> - !uid_eq(pcred->session_keyring->uid, mycred->euid)) ||
> - !uid_eq(mycred->session_keyring->uid, mycred->euid))
> - goto unlock;
> -
> - /* cancel an already pending keyring replacement */
> - oldwork = task_work_cancel_func(parent, key_change_session_keyring);
> -
> - /* the replacement session keyring is applied just prior to userspace
> - * restarting */
> - ret = task_work_add(parent, newwork, TWA_RESUME);
> - if (!ret)
> - newwork = NULL;
> -unlock:
> - write_unlock_irq(&tasklist_lock);
> - rcu_read_unlock();
> - if (oldwork)
> - put_cred(container_of(oldwork, struct cred, rcu));
> - if (newwork)
> - put_cred(cred);
> - return ret;
> -
> -error_keyring:
> +put_task:
> + put_task_struct(parent);
> key_ref_put(keyring_r);
> return ret;
> }
>
> /*
> * Apply a restriction to a given keyring.
> diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
> index b5d5333ab330..199c5dd34792 100644
> --- a/security/keys/process_keys.c
> +++ b/security/keys/process_keys.c
> @@ -902,59 +902,65 @@ long join_session_keyring(const char *name)
> error:
> abort_creds(new);
> return ret;
> }
>
> /*
> - * Replace a process's session keyring on behalf of one of its children when
> - * the target process is about to resume userspace execution.
> + * Replace a process's session keyring on behalf of one of its children.
> + * This function runs in task context, while the child is blocked in
> + * keyctl_session_to_parent().
> */
> -void key_change_session_keyring(struct callback_head *twork)
> +void key_change_session_keyring(struct callback_head *work)
> {
> - const struct cred *old = current_cred();
> - struct cred *new = container_of(twork, struct cred, rcu);
> + struct keyctl_session_to_parent_context *ctx =
> + container_of(work, struct keyctl_session_to_parent_context, work);
> + const struct cred *pcred = current_cred();
> + const struct cred *ccred = ctx->child_cred;
> + struct cred *new;
>
> - if (unlikely(current->flags & PF_EXITING)) {
> - put_cred(new);
> - return;
> - }
> + /* do checks */
> + ctx->result = -EPERM;
> + if (unlikely(current->flags & PF_EXITING))
> + goto out;
>
> - /* If get_ucounts fails more bits are needed in the refcount */
> - if (unlikely(!get_ucounts(old->ucounts))) {
> - WARN_ONCE(1, "In %s get_ucounts failed\n", __func__);
> - put_cred(new);
> - return;
> - }
> + /* we must be single threaded */
> + if (!thread_group_empty(current))
> + goto out;
> +
> + /*
> + * the parent must have the same effective ownership and mustn't be
> + * SUID/SGID
> + */
> + if (!uid_eq(pcred->uid, ccred->euid) ||
> + !uid_eq(pcred->euid, ccred->euid) ||
> + !uid_eq(pcred->suid, ccred->euid) ||
> + !gid_eq(pcred->gid, ccred->egid) ||
> + !gid_eq(pcred->egid, ccred->egid) ||
> + !gid_eq(pcred->sgid, ccred->egid))
> + goto out;
> +
> + /* the keyrings must have the same UID */
> + if ((pcred->session_keyring &&
> + !uid_eq(pcred->session_keyring->uid, ccred->euid)) ||
> + !uid_eq(ctx->new_session_keyring->uid, ccred->euid))
> + goto out;
> +
> +
> + /* okay, try to update creds */
> + ctx->result = -ENOMEM;
> + new = prepare_creds();
> + if (!new)
> + goto out;
>
> - new-> uid = old-> uid;
> - new-> euid = old-> euid;
> - new-> suid = old-> suid;
> - new->fsuid = old->fsuid;
> - new-> gid = old-> gid;
> - new-> egid = old-> egid;
> - new-> sgid = old-> sgid;
> - new->fsgid = old->fsgid;
> - new->user = get_uid(old->user);
> - new->ucounts = old->ucounts;
> - new->user_ns = get_user_ns(old->user_ns);
> - new->group_info = get_group_info(old->group_info);
> -
> - new->securebits = old->securebits;
> - new->cap_inheritable = old->cap_inheritable;
> - new->cap_permitted = old->cap_permitted;
> - new->cap_effective = old->cap_effective;
> - new->cap_ambient = old->cap_ambient;
> - new->cap_bset = old->cap_bset;
> -
> - new->jit_keyring = old->jit_keyring;
> - new->thread_keyring = key_get(old->thread_keyring);
> - new->process_keyring = key_get(old->process_keyring);
> -
> - security_transfer_creds(new, old);
> + key_put(new->session_keyring);
> + new->session_keyring = key_get(ctx->new_session_keyring);
>
> commit_creds(new);
> + ctx->result = 0;
> +out:
> + complete_all(&ctx->done);
> }
>
> /*
> * Make sure that root's user and user-session keyrings exist.
> */
> static int __init init_root_keyring(void)
> diff --git a/security/landlock/cred.c b/security/landlock/cred.c
> index db9fe7d906ba..786af18c4a1c 100644
> --- a/security/landlock/cred.c
> +++ b/security/landlock/cred.c
> @@ -11,41 +11,34 @@
>
> #include "common.h"
> #include "cred.h"
> #include "ruleset.h"
> #include "setup.h"
>
> -static void hook_cred_transfer(struct cred *const new,
> - const struct cred *const old)
> +static int hook_cred_prepare(struct cred *const new,
> + const struct cred *const old, const gfp_t gfp)
> {
> struct landlock_ruleset *const old_dom = landlock_cred(old)->domain;
>
> if (old_dom) {
> landlock_get_ruleset(old_dom);
> landlock_cred(new)->domain = old_dom;
> }
> -}
> -
> -static int hook_cred_prepare(struct cred *const new,
> - const struct cred *const old, const gfp_t gfp)
> -{
> - hook_cred_transfer(new, old);
> return 0;
> }
>
> static void hook_cred_free(struct cred *const cred)
> {
> struct landlock_ruleset *const dom = landlock_cred(cred)->domain;
>
> if (dom)
> landlock_put_ruleset_deferred(dom);
> }
>
> static struct security_hook_list landlock_hooks[] __ro_after_init = {
> LSM_HOOK_INIT(cred_prepare, hook_cred_prepare),
> - LSM_HOOK_INIT(cred_transfer, hook_cred_transfer),
> LSM_HOOK_INIT(cred_free, hook_cred_free),
> };
>
> __init void landlock_add_cred_hooks(void)
> {
> security_add_hooks(landlock_hooks, ARRAY_SIZE(landlock_hooks),
> diff --git a/security/security.c b/security/security.c
> index 8cee5b6c6e6d..4fb81de5cf80 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -3057,35 +3057,12 @@ void security_task_free(struct task_struct *task)
> call_void_hook(task_free, task);
>
> kfree(task->security);
> task->security = NULL;
> }
>
> -/**
> - * security_cred_alloc_blank() - Allocate the min memory to allow cred_transfer
> - * @cred: credentials
> - * @gfp: gfp flags
> - *
> - * Only allocate sufficient memory and attach to @cred such that
> - * cred_transfer() will not get ENOMEM.
> - *
> - * Return: Returns 0 on success, negative values on failure.
> - */
> -int security_cred_alloc_blank(struct cred *cred, gfp_t gfp)
> -{
> - int rc = lsm_cred_alloc(cred, gfp);
> -
> - if (rc)
> - return rc;
> -
> - rc = call_int_hook(cred_alloc_blank, cred, gfp);
> - if (unlikely(rc))
> - security_cred_free(cred);
> - return rc;
> -}
> -
> /**
> * security_cred_free() - Free the cred's LSM blob and associated resources
> * @cred: credentials
> *
> * Deallocate and clear the cred->security field in a set of credentials.
> */
> @@ -3124,24 +3101,12 @@ int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp)
> rc = call_int_hook(cred_prepare, new, old, gfp);
> if (unlikely(rc))
> security_cred_free(new);
> return rc;
> }
>
> -/**
> - * security_transfer_creds() - Transfer creds
> - * @new: target credentials
> - * @old: original credentials
> - *
> - * Transfer data from original creds to new creds.
> - */
> -void security_transfer_creds(struct cred *new, const struct cred *old)
> -{
> - call_void_hook(cred_transfer, new, old);
> -}
> -
> /**
> * security_cred_getsecid() - Get the secid from a set of credentials
> * @c: credentials
> * @secid: secid value
> *
> * Retrieve the security identifier of the cred structure @c. In case of
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 55c78c318ccd..8a659475cc12 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4007,23 +4007,12 @@ static int selinux_cred_prepare(struct cred *new, const struct cred *old,
> struct task_security_struct *tsec = selinux_cred(new);
>
> *tsec = *old_tsec;
> return 0;
> }
>
> -/*
> - * transfer the SELinux data to a blank set of creds
> - */
> -static void selinux_cred_transfer(struct cred *new, const struct cred *old)
> -{
> - const struct task_security_struct *old_tsec = selinux_cred(old);
> - struct task_security_struct *tsec = selinux_cred(new);
> -
> - *tsec = *old_tsec;
> -}
> -
> static void selinux_cred_getsecid(const struct cred *c, u32 *secid)
> {
> *secid = cred_sid(c);
> }
>
> /*
> @@ -7213,13 +7202,12 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
> LSM_HOOK_INIT(file_receive, selinux_file_receive),
>
> LSM_HOOK_INIT(file_open, selinux_file_open),
>
> LSM_HOOK_INIT(task_alloc, selinux_task_alloc),
> LSM_HOOK_INIT(cred_prepare, selinux_cred_prepare),
> - LSM_HOOK_INIT(cred_transfer, selinux_cred_transfer),
> LSM_HOOK_INIT(cred_getsecid, selinux_cred_getsecid),
> LSM_HOOK_INIT(kernel_act_as, selinux_kernel_act_as),
> LSM_HOOK_INIT(kernel_create_files_as, selinux_kernel_create_files_as),
> LSM_HOOK_INIT(kernel_module_request, selinux_kernel_module_request),
> LSM_HOOK_INIT(kernel_load_data, selinux_kernel_load_data),
> LSM_HOOK_INIT(kernel_read_file, selinux_kernel_read_file),
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 4164699cd4f6..4cc658deb08b 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -2050,27 +2050,12 @@ static int smack_file_open(struct file *file)
> }
>
> /*
> * Task hooks
> */
>
> -/**
> - * smack_cred_alloc_blank - "allocate" blank task-level security credentials
> - * @cred: the new credentials
> - * @gfp: the atomicity of any memory allocations
> - *
> - * Prepare a blank set of credentials for modification. This must allocate all
> - * the memory the LSM module might require such that cred_transfer() can
> - * complete without error.
> - */
> -static int smack_cred_alloc_blank(struct cred *cred, gfp_t gfp)
> -{
> - init_task_smack(smack_cred(cred), NULL, NULL);
> - return 0;
> -}
> -
>
> /**
> * smack_cred_free - "free" task-level security credentials
> * @cred: the credentials in question
> *
> */
> @@ -2113,27 +2098,12 @@ static int smack_cred_prepare(struct cred *new, const struct cred *old,
>
> rc = smk_copy_relabel(&new_tsp->smk_relabel, &old_tsp->smk_relabel,
> gfp);
> return rc;
> }
>
> -/**
> - * smack_cred_transfer - Transfer the old credentials to the new credentials
> - * @new: the new credentials
> - * @old: the original credentials
> - *
> - * Fill in a set of blank credentials from another set of credentials.
> - */
> -static void smack_cred_transfer(struct cred *new, const struct cred *old)
> -{
> - struct task_smack *old_tsp = smack_cred(old);
> - struct task_smack *new_tsp = smack_cred(new);
> -
> - init_task_smack(new_tsp, old_tsp->smk_task, old_tsp->smk_task);
> -}
> -
> /**
> * smack_cred_getsecid - get the secid corresponding to a creds structure
> * @cred: the object creds
> * @secid: where to put the result
> *
> * Sets the secid to contain a u32 version of the smack label.
> @@ -5107,16 +5077,14 @@ static struct security_hook_list smack_hooks[] __ro_after_init = {
> LSM_HOOK_INIT(file_set_fowner, smack_file_set_fowner),
> LSM_HOOK_INIT(file_send_sigiotask, smack_file_send_sigiotask),
> LSM_HOOK_INIT(file_receive, smack_file_receive),
>
> LSM_HOOK_INIT(file_open, smack_file_open),
>
> - LSM_HOOK_INIT(cred_alloc_blank, smack_cred_alloc_blank),
Not going through everything but can we e.g. make a separe SMACK patch
prepending?
> LSM_HOOK_INIT(cred_free, smack_cred_free),
> LSM_HOOK_INIT(cred_prepare, smack_cred_prepare),
> - LSM_HOOK_INIT(cred_transfer, smack_cred_transfer),
> LSM_HOOK_INIT(cred_getsecid, smack_cred_getsecid),
> LSM_HOOK_INIT(kernel_act_as, smack_kernel_act_as),
> LSM_HOOK_INIT(kernel_create_files_as, smack_kernel_create_files_as),
> LSM_HOOK_INIT(task_setpgid, smack_task_setpgid),
> LSM_HOOK_INIT(task_getpgid, smack_task_getpgid),
> LSM_HOOK_INIT(task_getsid, smack_task_getsid),
>
> ---
> base-commit: c0ecd6388360d930440cc5554026818895199923
> change-id: 20240802-remove-cred-transfer-493a3b696da2
BR, Jarkko
More information about the Linux-security-module-archive
mailing list