* [PATCH 1/2] selinux: move avdcache to per-task security blob
@ 2025-11-14 17:45 Stephen Smalley
2025-11-14 17:45 ` [PATCH 2/2] selinux: rename task_security_struct to cred_security_struct Stephen Smalley
2025-11-17 19:55 ` [PATCH 1/2] selinux: move avdcache to per-task security blob Paul Moore
0 siblings, 2 replies; 7+ messages in thread
From: Stephen Smalley @ 2025-11-14 17:45 UTC (permalink / raw)
To: selinux; +Cc: paul, omosnace, Stephen Smalley
The SELinux task_security_struct was originally per-task but later
migrated to per-cred when creds were first introduced to Linux. The
avdcache was meant to be per-task rather than per-cred; move it to a
new task_selinux_struct that is allocated per-task.
Fixes: 5d7ddc59b3d89b724a5aa8f30d0db94ff8d2d93f ("selinux: reduce path walk overhead")
Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
---
security/selinux/hooks.c | 35 +++++++++++++++++++------------
security/selinux/include/objsec.h | 13 ++++++++++--
2 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index a22b1920242f..0eea43e4a90c 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -212,11 +212,15 @@ static int selinux_lsm_notifier_avc_callback(u32 event)
static void cred_init_security(void)
{
struct task_security_struct *tsec;
+ struct task_selinux_struct *tsel;
/* NOTE: the lsm framework zeros out the buffer on allocation */
tsec = selinux_cred(unrcu_pointer(current->real_cred));
- tsec->osid = tsec->sid = tsec->avdcache.sid = SECINITSID_KERNEL;
+ tsec->osid = tsec->sid = SECINITSID_KERNEL;
+
+ tsel = selinux_task(current);
+ tsel->avdcache.sid = SECINITSID_KERNEL;
}
/*
@@ -3126,10 +3130,10 @@ static noinline int audit_inode_permission(struct inode *inode,
* Clear the task's AVD cache in @tsec and reset it to the current policy's
* and task's info.
*/
-static inline void task_avdcache_reset(struct task_security_struct *tsec)
+static inline void task_avdcache_reset(struct task_selinux_struct *tsec)
{
memset(&tsec->avdcache.dir, 0, sizeof(tsec->avdcache.dir));
- tsec->avdcache.sid = tsec->sid;
+ tsec->avdcache.sid = current_sid();
tsec->avdcache.seqno = avc_policy_seqno();
tsec->avdcache.dir_spot = TSEC_AVDC_DIR_SIZE - 1;
}
@@ -3143,7 +3147,7 @@ static inline void task_avdcache_reset(struct task_security_struct *tsec)
* Search @tsec for a AVD cache entry that matches @isec and return it to the
* caller via @avdc. Returns 0 if a match is found, negative values otherwise.
*/
-static inline int task_avdcache_search(struct task_security_struct *tsec,
+static inline int task_avdcache_search(struct task_selinux_struct *tsec,
struct inode_security_struct *isec,
struct avdc_entry **avdc)
{
@@ -3153,7 +3157,7 @@ static inline int task_avdcache_search(struct task_security_struct *tsec,
if (isec->sclass != SECCLASS_DIR)
return -ENOENT;
- if (unlikely(tsec->sid != tsec->avdcache.sid ||
+ if (unlikely(current_sid() != tsec->avdcache.sid ||
tsec->avdcache.seqno != avc_policy_seqno())) {
task_avdcache_reset(tsec);
return -ENOENT;
@@ -3183,7 +3187,7 @@ static inline int task_avdcache_search(struct task_security_struct *tsec,
* Update the AVD cache in @tsec with the @avdc and @audited info associated
* with @isec.
*/
-static inline void task_avdcache_update(struct task_security_struct *tsec,
+static inline void task_avdcache_update(struct task_selinux_struct *tsec,
struct inode_security_struct *isec,
struct av_decision *avd,
u32 audited)
@@ -3217,7 +3221,8 @@ static int selinux_inode_permission(struct inode *inode, int requested)
{
int mask;
u32 perms;
- struct task_security_struct *tsec;
+ u32 sid = current_sid();
+ struct task_selinux_struct *tsec;
struct inode_security_struct *isec;
struct avdc_entry *avdc;
int rc, rc2;
@@ -3229,8 +3234,8 @@ static int selinux_inode_permission(struct inode *inode, int requested)
if (!mask)
return 0;
- tsec = selinux_cred(current_cred());
- if (task_avdcache_permnoaudit(tsec))
+ tsec = selinux_task(current);
+ if (task_avdcache_permnoaudit(tsec, sid))
return 0;
isec = inode_security_rcu(inode, requested & MAY_NOT_BLOCK);
@@ -3250,7 +3255,7 @@ static int selinux_inode_permission(struct inode *inode, int requested)
struct av_decision avd;
/* Cache miss. */
- rc = avc_has_perm_noaudit(tsec->sid, isec->sid, isec->sclass,
+ rc = avc_has_perm_noaudit(sid, isec->sid, isec->sclass,
perms, 0, &avd);
audited = avc_audit_required(perms, &avd, rc,
(requested & MAY_ACCESS) ? FILE__AUDIT_ACCESS : 0,
@@ -3299,11 +3304,11 @@ static int selinux_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
static int selinux_inode_getattr(const struct path *path)
{
- struct task_security_struct *tsec;
+ struct task_selinux_struct *tsec;
- tsec = selinux_cred(current_cred());
+ tsec = selinux_task(current);
- if (task_avdcache_permnoaudit(tsec))
+ if (task_avdcache_permnoaudit(tsec, current_sid()))
return 0;
return path_has_perm(current_cred(), path, FILE__GETATTR);
@@ -4167,7 +4172,10 @@ static int selinux_task_alloc(struct task_struct *task,
u64 clone_flags)
{
u32 sid = current_sid();
+ struct task_selinux_struct *old_tsec = selinux_task(current);
+ struct task_selinux_struct *new_tsec = selinux_task(task);
+ *new_tsec = *old_tsec;
return avc_has_perm(sid, sid, SECCLASS_PROCESS, PROCESS__FORK, NULL);
}
@@ -7154,6 +7162,7 @@ static int selinux_bpf_token_create(struct bpf_token *token, union bpf_attr *att
struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
.lbs_cred = sizeof(struct task_security_struct),
+ .lbs_task = sizeof(struct task_selinux_struct),
.lbs_file = sizeof(struct file_security_struct),
.lbs_inode = sizeof(struct inode_security_struct),
.lbs_ipc = sizeof(struct ipc_security_struct),
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 2d5139c6d45b..1ac35ae5332f 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -44,6 +44,9 @@ struct task_security_struct {
u32 create_sid; /* fscreate SID */
u32 keycreate_sid; /* keycreate SID */
u32 sockcreate_sid; /* fscreate SID */
+} __randomize_layout;
+
+struct task_selinux_struct {
#define TSEC_AVDC_DIR_SIZE (1 << 2)
struct {
u32 sid; /* current SID for cached entries */
@@ -54,10 +57,11 @@ struct task_security_struct {
} avdcache;
} __randomize_layout;
-static inline bool task_avdcache_permnoaudit(struct task_security_struct *tsec)
+static inline bool task_avdcache_permnoaudit(struct task_selinux_struct *tsec,
+ u32 sid)
{
return (tsec->avdcache.permissive_neveraudit &&
- tsec->sid == tsec->avdcache.sid &&
+ sid == tsec->avdcache.sid &&
tsec->avdcache.seqno == avc_policy_seqno());
}
@@ -177,6 +181,11 @@ static inline struct task_security_struct *selinux_cred(const struct cred *cred)
return cred->security + selinux_blob_sizes.lbs_cred;
}
+static inline struct task_selinux_struct *selinux_task(const struct task_struct *task)
+{
+ return task->security + selinux_blob_sizes.lbs_task;
+}
+
static inline struct file_security_struct *selinux_file(const struct file *file)
{
return file->f_security + selinux_blob_sizes.lbs_file;
--
2.51.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/2] selinux: rename task_security_struct to cred_security_struct 2025-11-14 17:45 [PATCH 1/2] selinux: move avdcache to per-task security blob Stephen Smalley @ 2025-11-14 17:45 ` Stephen Smalley 2025-11-17 19:55 ` [PATCH 1/2] selinux: move avdcache to per-task security blob Paul Moore 1 sibling, 0 replies; 7+ messages in thread From: Stephen Smalley @ 2025-11-14 17:45 UTC (permalink / raw) To: selinux; +Cc: paul, omosnace, Stephen Smalley Before Linux had cred structures, the SELinux task_security_struct was per-task and although the structure was switched to being per-cred long ago, the name was never updated. This change renames it to cred_security_struct to avoid confusion with the separate per-task task_selinux_struct. No functional change. A further renaming change could be to rename all cred_security_struct variables from tsec to csec but that can be done separately if desired. Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com> --- security/selinux/hooks.c | 58 +++++++++++++++---------------- security/selinux/include/objsec.h | 6 ++-- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 0eea43e4a90c..74f38449cfc7 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -211,7 +211,7 @@ static int selinux_lsm_notifier_avc_callback(u32 event) */ static void cred_init_security(void) { - struct task_security_struct *tsec; + struct cred_security_struct *tsec; struct task_selinux_struct *tsel; /* NOTE: the lsm framework zeros out the buffer on allocation */ @@ -228,7 +228,7 @@ static void cred_init_security(void) */ static inline u32 cred_sid(const struct cred *cred) { - const struct task_security_struct *tsec; + const struct cred_security_struct *tsec; tsec = selinux_cred(cred); return tsec->sid; @@ -442,7 +442,7 @@ static int may_context_mount_sb_relabel(u32 sid, struct superblock_security_struct *sbsec, const struct cred *cred) { - const struct task_security_struct *tsec = selinux_cred(cred); + const struct cred_security_struct *tsec = selinux_cred(cred); int rc; rc = avc_has_perm(tsec->sid, sbsec->sid, SECCLASS_FILESYSTEM, @@ -459,7 +459,7 @@ static int may_context_mount_inode_relabel(u32 sid, struct superblock_security_struct *sbsec, const struct cred *cred) { - const struct task_security_struct *tsec = selinux_cred(cred); + const struct cred_security_struct *tsec = selinux_cred(cred); int rc; rc = avc_has_perm(tsec->sid, sbsec->sid, SECCLASS_FILESYSTEM, FILESYSTEM__RELABELFROM, NULL); @@ -1793,7 +1793,7 @@ static int file_has_perm(const struct cred *cred, * Determine the label for an inode that might be unioned. */ static int -selinux_determine_inode_label(const struct task_security_struct *tsec, +selinux_determine_inode_label(const struct cred_security_struct *tsec, struct inode *dir, const struct qstr *name, u16 tclass, u32 *_new_isid) @@ -1822,7 +1822,7 @@ static int may_create(struct inode *dir, struct dentry *dentry, u16 tclass) { - const struct task_security_struct *tsec = selinux_cred(current_cred()); + const struct cred_security_struct *tsec = selinux_cred(current_cred()); struct inode_security_struct *dsec; struct superblock_security_struct *sbsec; u32 sid, newsid; @@ -2256,8 +2256,8 @@ static u32 ptrace_parent_sid(void) } static int check_nnp_nosuid(const struct linux_binprm *bprm, - const struct task_security_struct *old_tsec, - const struct task_security_struct *new_tsec) + const struct cred_security_struct *old_tsec, + const struct cred_security_struct *new_tsec) { int nnp = (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS); int nosuid = !mnt_may_suid(bprm->file->f_path.mnt); @@ -2310,8 +2310,8 @@ static int check_nnp_nosuid(const struct linux_binprm *bprm, static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm) { - const struct task_security_struct *old_tsec; - struct task_security_struct *new_tsec; + const struct cred_security_struct *old_tsec; + struct cred_security_struct *new_tsec; struct inode_security_struct *isec; struct common_audit_data ad; struct inode *inode = file_inode(bprm->file); @@ -2492,7 +2492,7 @@ static inline void flush_unauthorized_files(const struct cred *cred, */ static void selinux_bprm_committing_creds(const struct linux_binprm *bprm) { - struct task_security_struct *new_tsec; + struct cred_security_struct *new_tsec; struct rlimit *rlim, *initrlim; int rc, i; @@ -2538,7 +2538,7 @@ static void selinux_bprm_committing_creds(const struct linux_binprm *bprm) */ static void selinux_bprm_committed_creds(const struct linux_binprm *bprm) { - const struct task_security_struct *tsec = selinux_cred(current_cred()); + const struct cred_security_struct *tsec = selinux_cred(current_cred()); u32 osid, sid; int rc; @@ -2920,7 +2920,7 @@ static int selinux_dentry_create_files_as(struct dentry *dentry, int mode, { u32 newsid; int rc; - struct task_security_struct *tsec; + struct cred_security_struct *tsec; rc = selinux_determine_inode_label(selinux_cred(old), d_inode(dentry->d_parent), name, @@ -2938,7 +2938,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, const struct qstr *qstr, struct xattr *xattrs, int *xattr_count) { - const struct task_security_struct *tsec = selinux_cred(current_cred()); + const struct cred_security_struct *tsec = selinux_cred(current_cred()); struct superblock_security_struct *sbsec; struct xattr *xattr = lsm_get_xattr_slot(xattrs, xattr_count); u32 newsid, clen; @@ -3680,7 +3680,7 @@ static void selinux_inode_getlsmprop(struct inode *inode, struct lsm_prop *prop) static int selinux_inode_copy_up(struct dentry *src, struct cred **new) { struct lsm_prop prop; - struct task_security_struct *tsec; + struct cred_security_struct *tsec; struct cred *new_creds = *new; if (new_creds == NULL) { @@ -3718,7 +3718,7 @@ static int selinux_inode_copy_up_xattr(struct dentry *dentry, const char *name) static int selinux_kernfs_init_security(struct kernfs_node *kn_dir, struct kernfs_node *kn) { - const struct task_security_struct *tsec = selinux_cred(current_cred()); + const struct cred_security_struct *tsec = selinux_cred(current_cred()); u32 parent_sid, newsid, clen; int rc; char *context; @@ -4185,8 +4185,8 @@ static int selinux_task_alloc(struct task_struct *task, static int selinux_cred_prepare(struct cred *new, const struct cred *old, gfp_t gfp) { - const struct task_security_struct *old_tsec = selinux_cred(old); - struct task_security_struct *tsec = selinux_cred(new); + const struct cred_security_struct *old_tsec = selinux_cred(old); + struct cred_security_struct *tsec = selinux_cred(new); *tsec = *old_tsec; return 0; @@ -4197,8 +4197,8 @@ static int selinux_cred_prepare(struct cred *new, const struct cred *old, */ 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); + const struct cred_security_struct *old_tsec = selinux_cred(old); + struct cred_security_struct *tsec = selinux_cred(new); *tsec = *old_tsec; } @@ -4219,7 +4219,7 @@ static void selinux_cred_getlsmprop(const struct cred *c, struct lsm_prop *prop) */ static int selinux_kernel_act_as(struct cred *new, u32 secid) { - struct task_security_struct *tsec = selinux_cred(new); + struct cred_security_struct *tsec = selinux_cred(new); u32 sid = current_sid(); int ret; @@ -4243,7 +4243,7 @@ static int selinux_kernel_act_as(struct cred *new, u32 secid) static int selinux_kernel_create_files_as(struct cred *new, struct inode *inode) { struct inode_security_struct *isec = inode_security(inode); - struct task_security_struct *tsec = selinux_cred(new); + struct cred_security_struct *tsec = selinux_cred(new); u32 sid = current_sid(); int ret; @@ -4768,7 +4768,7 @@ static int selinux_conn_sid(u32 sk_sid, u32 skb_sid, u32 *conn_sid) /* socket security operations */ -static int socket_sockcreate_sid(const struct task_security_struct *tsec, +static int socket_sockcreate_sid(const struct cred_security_struct *tsec, u16 secclass, u32 *socksid) { if (tsec->sockcreate_sid > SECSID_NULL) { @@ -4821,7 +4821,7 @@ static int sock_has_perm(struct sock *sk, u32 perms) static int selinux_socket_create(int family, int type, int protocol, int kern) { - const struct task_security_struct *tsec = selinux_cred(current_cred()); + const struct cred_security_struct *tsec = selinux_cred(current_cred()); u32 newsid; u16 secclass; int rc; @@ -4840,7 +4840,7 @@ static int selinux_socket_create(int family, int type, static int selinux_socket_post_create(struct socket *sock, int family, int type, int protocol, int kern) { - const struct task_security_struct *tsec = selinux_cred(current_cred()); + const struct cred_security_struct *tsec = selinux_cred(current_cred()); struct inode_security_struct *isec = inode_security_novalidate(SOCK_INODE(sock)); struct sk_security_struct *sksec; u16 sclass = socket_type_to_security_class(family, type, protocol); @@ -6550,7 +6550,7 @@ static void selinux_d_instantiate(struct dentry *dentry, struct inode *inode) static int selinux_lsm_getattr(unsigned int attr, struct task_struct *p, char **value) { - const struct task_security_struct *tsec; + const struct cred_security_struct *tsec; int error; u32 sid; u32 len; @@ -6605,7 +6605,7 @@ static int selinux_lsm_getattr(unsigned int attr, struct task_struct *p, static int selinux_lsm_setattr(u64 attr, void *value, size_t size) { - struct task_security_struct *tsec; + struct cred_security_struct *tsec; struct cred *new; u32 mysid = current_sid(), sid = 0, ptsid; int error; @@ -6900,7 +6900,7 @@ static int selinux_inode_getsecctx(struct inode *inode, struct lsm_context *cp) static int selinux_key_alloc(struct key *k, const struct cred *cred, unsigned long flags) { - const struct task_security_struct *tsec; + const struct cred_security_struct *tsec; struct key_security_struct *ksec = selinux_key(k); tsec = selinux_cred(cred); @@ -7161,7 +7161,7 @@ static int selinux_bpf_token_create(struct bpf_token *token, union bpf_attr *att #endif struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = { - .lbs_cred = sizeof(struct task_security_struct), + .lbs_cred = sizeof(struct cred_security_struct), .lbs_task = sizeof(struct task_selinux_struct), .lbs_file = sizeof(struct file_security_struct), .lbs_inode = sizeof(struct inode_security_struct), diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h index 1ac35ae5332f..b90f842cc310 100644 --- a/security/selinux/include/objsec.h +++ b/security/selinux/include/objsec.h @@ -37,7 +37,7 @@ struct avdc_entry { bool permissive; /* AVC permissive flag */ }; -struct task_security_struct { +struct cred_security_struct { u32 osid; /* SID prior to last execve */ u32 sid; /* current SID */ u32 exec_sid; /* exec SID */ @@ -176,7 +176,7 @@ struct perf_event_security_struct { }; extern struct lsm_blob_sizes selinux_blob_sizes; -static inline struct task_security_struct *selinux_cred(const struct cred *cred) +static inline struct cred_security_struct *selinux_cred(const struct cred *cred) { return cred->security + selinux_blob_sizes.lbs_cred; } @@ -216,7 +216,7 @@ selinux_ipc(const struct kern_ipc_perm *ipc) */ static inline u32 current_sid(void) { - const struct task_security_struct *tsec = selinux_cred(current_cred()); + const struct cred_security_struct *tsec = selinux_cred(current_cred()); return tsec->sid; } -- 2.51.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] selinux: move avdcache to per-task security blob 2025-11-14 17:45 [PATCH 1/2] selinux: move avdcache to per-task security blob Stephen Smalley 2025-11-14 17:45 ` [PATCH 2/2] selinux: rename task_security_struct to cred_security_struct Stephen Smalley @ 2025-11-17 19:55 ` Paul Moore 2025-11-17 21:25 ` Stephen Smalley 1 sibling, 1 reply; 7+ messages in thread From: Paul Moore @ 2025-11-17 19:55 UTC (permalink / raw) To: Stephen Smalley; +Cc: selinux, omosnace On Fri, Nov 14, 2025 at 12:45 PM Stephen Smalley <stephen.smalley.work@gmail.com> wrote: > > The SELinux task_security_struct was originally per-task but later > migrated to per-cred when creds were first introduced to Linux. The > avdcache was meant to be per-task rather than per-cred; move it to a > new task_selinux_struct that is allocated per-task. > > Fixes: 5d7ddc59b3d89b724a5aa8f30d0db94ff8d2d93f ("selinux: reduce path walk overhead") > Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com> > --- > security/selinux/hooks.c | 35 +++++++++++++++++++------------ > security/selinux/include/objsec.h | 13 ++++++++++-- > 2 files changed, 33 insertions(+), 15 deletions(-) Apologies for the delay, I was away the past few days. > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index a22b1920242f..0eea43e4a90c 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -212,11 +212,15 @@ static int selinux_lsm_notifier_avc_callback(u32 event) > static void cred_init_security(void) > { > struct task_security_struct *tsec; > + struct task_selinux_struct *tsel; I know we had a brief discussion about this off-list before you posted the patchset and I asked you to move the task/cred renaming patch after this one, but looking at the results in cred_init_security() I'm regretting that comment; the naming is just too ugly otherwise. As this patch shuffling is my fault, I'll go ahead and re-arrange them and follow up with a reply when they are in the stable-6.18 tree so you can verify everything is still okay with you. > /* NOTE: the lsm framework zeros out the buffer on allocation */ > > tsec = selinux_cred(unrcu_pointer(current->real_cred)); > - tsec->osid = tsec->sid = tsec->avdcache.sid = SECINITSID_KERNEL; > + tsec->osid = tsec->sid = SECINITSID_KERNEL; > + > + tsel = selinux_task(current); > + tsel->avdcache.sid = SECINITSID_KERNEL; > } -- paul-moore.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] selinux: move avdcache to per-task security blob 2025-11-17 19:55 ` [PATCH 1/2] selinux: move avdcache to per-task security blob Paul Moore @ 2025-11-17 21:25 ` Stephen Smalley 2025-11-17 21:43 ` Paul Moore 0 siblings, 1 reply; 7+ messages in thread From: Stephen Smalley @ 2025-11-17 21:25 UTC (permalink / raw) To: Paul Moore; +Cc: selinux, omosnace On Mon, Nov 17, 2025 at 2:55 PM Paul Moore <paul@paul-moore.com> wrote: > > On Fri, Nov 14, 2025 at 12:45 PM Stephen Smalley > <stephen.smalley.work@gmail.com> wrote: > > > > The SELinux task_security_struct was originally per-task but later > > migrated to per-cred when creds were first introduced to Linux. The > > avdcache was meant to be per-task rather than per-cred; move it to a > > new task_selinux_struct that is allocated per-task. > > > > Fixes: 5d7ddc59b3d89b724a5aa8f30d0db94ff8d2d93f ("selinux: reduce path walk overhead") > > Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com> > > --- > > security/selinux/hooks.c | 35 +++++++++++++++++++------------ > > security/selinux/include/objsec.h | 13 ++++++++++-- > > 2 files changed, 33 insertions(+), 15 deletions(-) > > Apologies for the delay, I was away the past few days. > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > index a22b1920242f..0eea43e4a90c 100644 > > --- a/security/selinux/hooks.c > > +++ b/security/selinux/hooks.c > > @@ -212,11 +212,15 @@ static int selinux_lsm_notifier_avc_callback(u32 event) > > static void cred_init_security(void) > > { > > struct task_security_struct *tsec; > > + struct task_selinux_struct *tsel; > > I know we had a brief discussion about this off-list before you posted > the patchset and I asked you to move the task/cred renaming patch > after this one, but looking at the results in cred_init_security() I'm > regretting that comment; the naming is just too ugly otherwise. I'm not 100% sure that setting avdcache.sid in cred_init_security() is even necessary since it should be set whenever the avdcache is first populated and there is nothing else being initialized in the avdcache. So if that's the only ugly part we could just drop it. I only included it since it was being done previously. It wasn't in my original patches. > As this patch shuffling is my fault, I'll go ahead and re-arrange them > and follow up with a reply when they are in the stable-6.18 tree so > you can verify everything is still okay with you. In that case I'd be inclined to revert to the original patches since they are already ordered correctly and they don't introduce the extra task_selinux_struct name since it isn't needed. > > > /* NOTE: the lsm framework zeros out the buffer on allocation */ > > > > tsec = selinux_cred(unrcu_pointer(current->real_cred)); > > - tsec->osid = tsec->sid = tsec->avdcache.sid = SECINITSID_KERNEL; > > + tsec->osid = tsec->sid = SECINITSID_KERNEL; > > + > > + tsel = selinux_task(current); > > + tsel->avdcache.sid = SECINITSID_KERNEL; > > } > > -- > paul-moore.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] selinux: move avdcache to per-task security blob 2025-11-17 21:25 ` Stephen Smalley @ 2025-11-17 21:43 ` Paul Moore 2025-11-18 13:44 ` Stephen Smalley 0 siblings, 1 reply; 7+ messages in thread From: Paul Moore @ 2025-11-17 21:43 UTC (permalink / raw) To: Stephen Smalley; +Cc: selinux, omosnace On Mon, Nov 17, 2025 at 4:26 PM Stephen Smalley <stephen.smalley.work@gmail.com> wrote: > On Mon, Nov 17, 2025 at 2:55 PM Paul Moore <paul@paul-moore.com> wrote: > > > > On Fri, Nov 14, 2025 at 12:45 PM Stephen Smalley > > <stephen.smalley.work@gmail.com> wrote: > > > > > > The SELinux task_security_struct was originally per-task but later > > > migrated to per-cred when creds were first introduced to Linux. The > > > avdcache was meant to be per-task rather than per-cred; move it to a > > > new task_selinux_struct that is allocated per-task. > > > > > > Fixes: 5d7ddc59b3d89b724a5aa8f30d0db94ff8d2d93f ("selinux: reduce path walk overhead") > > > Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com> > > > --- > > > security/selinux/hooks.c | 35 +++++++++++++++++++------------ > > > security/selinux/include/objsec.h | 13 ++++++++++-- > > > 2 files changed, 33 insertions(+), 15 deletions(-) > > > > Apologies for the delay, I was away the past few days. > > > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > > index a22b1920242f..0eea43e4a90c 100644 > > > --- a/security/selinux/hooks.c > > > +++ b/security/selinux/hooks.c > > > @@ -212,11 +212,15 @@ static int selinux_lsm_notifier_avc_callback(u32 event) > > > static void cred_init_security(void) > > > { > > > struct task_security_struct *tsec; > > > + struct task_selinux_struct *tsel; > > > > I know we had a brief discussion about this off-list before you posted > > the patchset and I asked you to move the task/cred renaming patch > > after this one, but looking at the results in cred_init_security() I'm > > regretting that comment; the naming is just too ugly otherwise. > > I'm not 100% sure that setting avdcache.sid in cred_init_security() is > even necessary since it should be set whenever the avdcache is first > populated and there is nothing else being initialized in the avdcache. > So if that's the only ugly part we could just drop it. I only included > it since it was being done previously. It wasn't in my original > patches. > > > As this patch shuffling is my fault, I'll go ahead and re-arrange them > > and follow up with a reply when they are in the stable-6.18 tree so > > you can verify everything is still okay with you. > > In that case I'd be inclined to revert to the original patches since > they are already ordered correctly and they don't introduce the extra > task_selinux_struct name since it isn't needed. I'd want to go back and look at those patches again, but I don't recall anything jumping out at me during the quick review. I did just check and they do have your sign-off, do you have a problem if I merge those (adding the fixes tag, etc.)? I'll resend them to the list simply so we have a record of that beyond just the git repo. Or did you want to change anything in there first? -- paul-moore.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] selinux: move avdcache to per-task security blob 2025-11-17 21:43 ` Paul Moore @ 2025-11-18 13:44 ` Stephen Smalley 2025-11-18 15:47 ` Paul Moore 0 siblings, 1 reply; 7+ messages in thread From: Stephen Smalley @ 2025-11-18 13:44 UTC (permalink / raw) To: Paul Moore; +Cc: selinux, omosnace On Mon, Nov 17, 2025 at 4:43 PM Paul Moore <paul@paul-moore.com> wrote: > > On Mon, Nov 17, 2025 at 4:26 PM Stephen Smalley > <stephen.smalley.work@gmail.com> wrote: > > On Mon, Nov 17, 2025 at 2:55 PM Paul Moore <paul@paul-moore.com> wrote: > > > > > > On Fri, Nov 14, 2025 at 12:45 PM Stephen Smalley > > > <stephen.smalley.work@gmail.com> wrote: > > > > > > > > The SELinux task_security_struct was originally per-task but later > > > > migrated to per-cred when creds were first introduced to Linux. The > > > > avdcache was meant to be per-task rather than per-cred; move it to a > > > > new task_selinux_struct that is allocated per-task. > > > > > > > > Fixes: 5d7ddc59b3d89b724a5aa8f30d0db94ff8d2d93f ("selinux: reduce path walk overhead") > > > > Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com> > > > > --- > > > > security/selinux/hooks.c | 35 +++++++++++++++++++------------ > > > > security/selinux/include/objsec.h | 13 ++++++++++-- > > > > 2 files changed, 33 insertions(+), 15 deletions(-) > > > > > > Apologies for the delay, I was away the past few days. > > > > > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > > > index a22b1920242f..0eea43e4a90c 100644 > > > > --- a/security/selinux/hooks.c > > > > +++ b/security/selinux/hooks.c > > > > @@ -212,11 +212,15 @@ static int selinux_lsm_notifier_avc_callback(u32 event) > > > > static void cred_init_security(void) > > > > { > > > > struct task_security_struct *tsec; > > > > + struct task_selinux_struct *tsel; > > > > > > I know we had a brief discussion about this off-list before you posted > > > the patchset and I asked you to move the task/cred renaming patch > > > after this one, but looking at the results in cred_init_security() I'm > > > regretting that comment; the naming is just too ugly otherwise. > > > > I'm not 100% sure that setting avdcache.sid in cred_init_security() is > > even necessary since it should be set whenever the avdcache is first > > populated and there is nothing else being initialized in the avdcache. > > So if that's the only ugly part we could just drop it. I only included > > it since it was being done previously. It wasn't in my original > > patches. > > > > > As this patch shuffling is my fault, I'll go ahead and re-arrange them > > > and follow up with a reply when they are in the stable-6.18 tree so > > > you can verify everything is still okay with you. > > > > In that case I'd be inclined to revert to the original patches since > > they are already ordered correctly and they don't introduce the extra > > task_selinux_struct name since it isn't needed. > > I'd want to go back and look at those patches again, but I don't > recall anything jumping out at me during the quick review. I did just > check and they do have your sign-off, do you have a problem if I merge > those (adding the fixes tag, etc.)? I'll resend them to the list > simply so we have a record of that beyond just the git repo. Or did > you want to change anything in there first? That's fine with me. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] selinux: move avdcache to per-task security blob 2025-11-18 13:44 ` Stephen Smalley @ 2025-11-18 15:47 ` Paul Moore 0 siblings, 0 replies; 7+ messages in thread From: Paul Moore @ 2025-11-18 15:47 UTC (permalink / raw) To: Stephen Smalley; +Cc: selinux, omosnace On Tue, Nov 18, 2025 at 8:44 AM Stephen Smalley <stephen.smalley.work@gmail.com> wrote: > On Mon, Nov 17, 2025 at 4:43 PM Paul Moore <paul@paul-moore.com> wrote: > > On Mon, Nov 17, 2025 at 4:26 PM Stephen Smalley > > <stephen.smalley.work@gmail.com> wrote: > > > On Mon, Nov 17, 2025 at 2:55 PM Paul Moore <paul@paul-moore.com> wrote: > > > > > > > > On Fri, Nov 14, 2025 at 12:45 PM Stephen Smalley > > > > <stephen.smalley.work@gmail.com> wrote: > > > > > > > > > > The SELinux task_security_struct was originally per-task but later > > > > > migrated to per-cred when creds were first introduced to Linux. The > > > > > avdcache was meant to be per-task rather than per-cred; move it to a > > > > > new task_selinux_struct that is allocated per-task. > > > > > > > > > > Fixes: 5d7ddc59b3d89b724a5aa8f30d0db94ff8d2d93f ("selinux: reduce path walk overhead") > > > > > Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com> > > > > > --- > > > > > security/selinux/hooks.c | 35 +++++++++++++++++++------------ > > > > > security/selinux/include/objsec.h | 13 ++++++++++-- > > > > > 2 files changed, 33 insertions(+), 15 deletions(-) > > > > > > > > Apologies for the delay, I was away the past few days. > > > > > > > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > > > > index a22b1920242f..0eea43e4a90c 100644 > > > > > --- a/security/selinux/hooks.c > > > > > +++ b/security/selinux/hooks.c > > > > > @@ -212,11 +212,15 @@ static int selinux_lsm_notifier_avc_callback(u32 event) > > > > > static void cred_init_security(void) > > > > > { > > > > > struct task_security_struct *tsec; > > > > > + struct task_selinux_struct *tsel; > > > > > > > > I know we had a brief discussion about this off-list before you posted > > > > the patchset and I asked you to move the task/cred renaming patch > > > > after this one, but looking at the results in cred_init_security() I'm > > > > regretting that comment; the naming is just too ugly otherwise. > > > > > > I'm not 100% sure that setting avdcache.sid in cred_init_security() is > > > even necessary since it should be set whenever the avdcache is first > > > populated and there is nothing else being initialized in the avdcache. > > > So if that's the only ugly part we could just drop it. I only included > > > it since it was being done previously. It wasn't in my original > > > patches. > > > > > > > As this patch shuffling is my fault, I'll go ahead and re-arrange them > > > > and follow up with a reply when they are in the stable-6.18 tree so > > > > you can verify everything is still okay with you. > > > > > > In that case I'd be inclined to revert to the original patches since > > > they are already ordered correctly and they don't introduce the extra > > > task_selinux_struct name since it isn't needed. > > > > I'd want to go back and look at those patches again, but I don't > > recall anything jumping out at me during the quick review. I did just > > check and they do have your sign-off, do you have a problem if I merge > > those (adding the fixes tag, etc.)? I'll resend them to the list > > simply so we have a record of that beyond just the git repo. Or did > > you want to change anything in there first? > > That's fine with me. Okay, expect that later today. -- paul-moore.com ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-11-18 15:47 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-14 17:45 [PATCH 1/2] selinux: move avdcache to per-task security blob Stephen Smalley 2025-11-14 17:45 ` [PATCH 2/2] selinux: rename task_security_struct to cred_security_struct Stephen Smalley 2025-11-17 19:55 ` [PATCH 1/2] selinux: move avdcache to per-task security blob Paul Moore 2025-11-17 21:25 ` Stephen Smalley 2025-11-17 21:43 ` Paul Moore 2025-11-18 13:44 ` Stephen Smalley 2025-11-18 15:47 ` Paul Moore
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).