Skip to content

Commit 87cc7b0

Browse files
mjguzikjrjohansen
mjguzik
authored and
jrjohansen
committed
apparmor: make __begin_current_label_crit_section() indicate whether put is needed
Same as aa_get_newest_cred_label_condref(). This avoids a bunch of work overall and allows the compiler to note when no clean up is necessary, allowing for tail calls. This in particular happens in apparmor_file_permission(), which manages to tail call aa_file_perm() 105 bytes in (vs a regular call 112 bytes in followed by branches to figure out if clean up is needed). Signed-off-by: Mateusz Guzik <[email protected]> Signed-off-by: John Johansen <[email protected]>
1 parent 37a3741 commit 87cc7b0

File tree

3 files changed

+67
-41
lines changed

3 files changed

+67
-41
lines changed

‎security/apparmor/include/cred.h

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,12 @@ static inline struct aa_label *aa_get_current_label(void)
114114
return aa_get_label(l);
115115
}
116116

117-
#define __end_current_label_crit_section(X) end_current_label_crit_section(X)
117+
static inline void __end_current_label_crit_section(struct aa_label *label,
118+
bool needput)
119+
{
120+
if (unlikely(needput))
121+
aa_put_label(label);
122+
}
118123

119124
/**
120125
* end_current_label_crit_section - put a reference found with begin_current_label..
@@ -142,13 +147,16 @@ static inline void end_current_label_crit_section(struct aa_label *label)
142147
* critical section between __begin_current_label_crit_section() ..
143148
* __end_current_label_crit_section()
144149
*/
145-
static inline struct aa_label *__begin_current_label_crit_section(void)
150+
static inline struct aa_label *__begin_current_label_crit_section(bool *needput)
146151
{
147152
struct aa_label *label = aa_current_raw_label();
148153

149-
if (label_is_stale(label))
150-
label = aa_get_newest_label(label);
154+
if (label_is_stale(label)) {
155+
*needput = true;
156+
return aa_get_newest_label(label);
157+
}
151158

159+
*needput = false;
152160
return label;
153161
}
154162

@@ -184,10 +192,11 @@ static inline struct aa_ns *aa_get_current_ns(void)
184192
{
185193
struct aa_label *label;
186194
struct aa_ns *ns;
195+
bool needput;
187196

188-
label = __begin_current_label_crit_section();
197+
label = __begin_current_label_crit_section(&needput);
189198
ns = aa_get_ns(labels_ns(label));
190-
__end_current_label_crit_section(label);
199+
__end_current_label_crit_section(label, needput);
191200

192201
return ns;
193202
}

‎security/apparmor/lsm.c

Lines changed: 46 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -127,14 +127,15 @@ static int apparmor_ptrace_access_check(struct task_struct *child,
127127
struct aa_label *tracer, *tracee;
128128
const struct cred *cred;
129129
int error;
130+
bool needput;
130131

131132
cred = get_task_cred(child);
132133
tracee = cred_label(cred); /* ref count on cred */
133-
tracer = __begin_current_label_crit_section();
134+
tracer = __begin_current_label_crit_section(&needput);
134135
error = aa_may_ptrace(current_cred(), tracer, cred, tracee,
135136
(mode & PTRACE_MODE_READ) ? AA_PTRACE_READ
136137
: AA_PTRACE_TRACE);
137-
__end_current_label_crit_section(tracer);
138+
__end_current_label_crit_section(tracer, needput);
138139
put_cred(cred);
139140

140141
return error;
@@ -145,14 +146,15 @@ static int apparmor_ptrace_traceme(struct task_struct *parent)
145146
struct aa_label *tracer, *tracee;
146147
const struct cred *cred;
147148
int error;
149+
bool needput;
148150

149-
tracee = __begin_current_label_crit_section();
151+
tracee = __begin_current_label_crit_section(&needput);
150152
cred = get_task_cred(parent);
151153
tracer = cred_label(cred); /* ref count on cred */
152154
error = aa_may_ptrace(cred, tracer, current_cred(), tracee,
153155
AA_PTRACE_TRACE);
154156
put_cred(cred);
155-
__end_current_label_crit_section(tracee);
157+
__end_current_label_crit_section(tracee, needput);
156158

157159
return error;
158160
}
@@ -221,12 +223,13 @@ static int common_perm(const char *op, const struct path *path, u32 mask,
221223
{
222224
struct aa_label *label;
223225
int error = 0;
226+
bool needput;
224227

225-
label = __begin_current_label_crit_section();
228+
label = __begin_current_label_crit_section(&needput);
226229
if (!unconfined(label))
227230
error = aa_path_perm(op, current_cred(), label, path, 0, mask,
228231
cond);
229-
__end_current_label_crit_section(label);
232+
__end_current_label_crit_section(label, needput);
230233

231234
return error;
232235
}
@@ -524,14 +527,15 @@ static int common_file_perm(const char *op, struct file *file, u32 mask,
524527
{
525528
struct aa_label *label;
526529
int error = 0;
530+
bool needput;
527531

528532
/* don't reaudit files closed during inheritance */
529-
if (file->f_path.dentry == aa_null.dentry)
533+
if (unlikely(file->f_path.dentry == aa_null.dentry))
530534
return -EACCES;
531535

532-
label = __begin_current_label_crit_section();
536+
label = __begin_current_label_crit_section(&needput);
533537
error = aa_file_perm(op, current_cred(), label, file, mask, in_atomic);
534-
__end_current_label_crit_section(label);
538+
__end_current_label_crit_section(label, needput);
535539

536540
return error;
537541
}
@@ -664,15 +668,16 @@ static int apparmor_uring_override_creds(const struct cred *new)
664668
struct aa_profile *profile;
665669
struct aa_label *label;
666670
int error;
671+
bool needput;
667672
DEFINE_AUDIT_DATA(ad, LSM_AUDIT_DATA_NONE, AA_CLASS_IO_URING,
668673
OP_URING_OVERRIDE);
669674

670675
ad.uring.target = cred_label(new);
671-
label = __begin_current_label_crit_section();
676+
label = __begin_current_label_crit_section(&needput);
672677
error = fn_for_each(label, profile,
673678
profile_uring(profile, AA_MAY_OVERRIDE_CRED,
674679
cred_label(new), CAP_SYS_ADMIN, &ad));
675-
__end_current_label_crit_section(label);
680+
__end_current_label_crit_section(label, needput);
676681

677682
return error;
678683
}
@@ -688,14 +693,15 @@ static int apparmor_uring_sqpoll(void)
688693
struct aa_profile *profile;
689694
struct aa_label *label;
690695
int error;
696+
bool needput;
691697
DEFINE_AUDIT_DATA(ad, LSM_AUDIT_DATA_NONE, AA_CLASS_IO_URING,
692698
OP_URING_SQPOLL);
693699

694-
label = __begin_current_label_crit_section();
700+
label = __begin_current_label_crit_section(&needput);
695701
error = fn_for_each(label, profile,
696702
profile_uring(profile, AA_MAY_CREATE_SQPOLL,
697703
NULL, CAP_SYS_ADMIN, &ad));
698-
__end_current_label_crit_section(label);
704+
__end_current_label_crit_section(label, needput);
699705

700706
return error;
701707
}
@@ -706,14 +712,15 @@ static int apparmor_sb_mount(const char *dev_name, const struct path *path,
706712
{
707713
struct aa_label *label;
708714
int error = 0;
715+
bool needput;
709716

710717
/* Discard magic */
711718
if ((flags & MS_MGC_MSK) == MS_MGC_VAL)
712719
flags &= ~MS_MGC_MSK;
713720

714721
flags &= ~AA_MS_IGNORE_MASK;
715722

716-
label = __begin_current_label_crit_section();
723+
label = __begin_current_label_crit_section(&needput);
717724
if (!unconfined(label)) {
718725
if (flags & MS_REMOUNT)
719726
error = aa_remount(current_cred(), label, path, flags,
@@ -732,7 +739,7 @@ static int apparmor_sb_mount(const char *dev_name, const struct path *path,
732739
error = aa_new_mount(current_cred(), label, dev_name,
733740
path, type, flags, data);
734741
}
735-
__end_current_label_crit_section(label);
742+
__end_current_label_crit_section(label, needput);
736743

737744
return error;
738745
}
@@ -742,12 +749,13 @@ static int apparmor_move_mount(const struct path *from_path,
742749
{
743750
struct aa_label *label;
744751
int error = 0;
752+
bool needput;
745753

746-
label = __begin_current_label_crit_section();
754+
label = __begin_current_label_crit_section(&needput);
747755
if (!unconfined(label))
748756
error = aa_move_mount(current_cred(), label, from_path,
749757
to_path);
750-
__end_current_label_crit_section(label);
758+
__end_current_label_crit_section(label, needput);
751759

752760
return error;
753761
}
@@ -756,11 +764,12 @@ static int apparmor_sb_umount(struct vfsmount *mnt, int flags)
756764
{
757765
struct aa_label *label;
758766
int error = 0;
767+
bool needput;
759768

760-
label = __begin_current_label_crit_section();
769+
label = __begin_current_label_crit_section(&needput);
761770
if (!unconfined(label))
762771
error = aa_umount(current_cred(), label, mnt, flags);
763-
__end_current_label_crit_section(label);
772+
__end_current_label_crit_section(label, needput);
764773

765774
return error;
766775
}
@@ -984,10 +993,12 @@ static void apparmor_bprm_committed_creds(const struct linux_binprm *bprm)
984993

985994
static void apparmor_current_getlsmprop_subj(struct lsm_prop *prop)
986995
{
987-
struct aa_label *label = __begin_current_label_crit_section();
996+
struct aa_label *label;
997+
bool needput;
988998

999+
label = __begin_current_label_crit_section(&needput);
9891000
prop->apparmor.label = label;
990-
__end_current_label_crit_section(label);
1001+
__end_current_label_crit_section(label, needput);
9911002
}
9921003

9931004
static void apparmor_task_getlsmprop_obj(struct task_struct *p,
@@ -1002,13 +1013,16 @@ static void apparmor_task_getlsmprop_obj(struct task_struct *p,
10021013
static int apparmor_task_setrlimit(struct task_struct *task,
10031014
unsigned int resource, struct rlimit *new_rlim)
10041015
{
1005-
struct aa_label *label = __begin_current_label_crit_section();
1016+
struct aa_label *label;
10061017
int error = 0;
1018+
bool needput;
1019+
1020+
label = __begin_current_label_crit_section(&needput);
10071021

10081022
if (!unconfined(label))
10091023
error = aa_task_setrlimit(current_cred(), label, task,
10101024
resource, new_rlim);
1011-
__end_current_label_crit_section(label);
1025+
__end_current_label_crit_section(label, needput);
10121026

10131027
return error;
10141028
}
@@ -1019,6 +1033,7 @@ static int apparmor_task_kill(struct task_struct *target, struct kernel_siginfo
10191033
const struct cred *tc;
10201034
struct aa_label *cl, *tl;
10211035
int error;
1036+
bool needput;
10221037

10231038
tc = get_task_cred(target);
10241039
tl = aa_get_newest_cred_label(tc);
@@ -1030,9 +1045,9 @@ static int apparmor_task_kill(struct task_struct *target, struct kernel_siginfo
10301045
error = aa_may_signal(cred, cl, tc, tl, sig);
10311046
aa_put_label(cl);
10321047
} else {
1033-
cl = __begin_current_label_crit_section();
1048+
cl = __begin_current_label_crit_section(&needput);
10341049
error = aa_may_signal(current_cred(), cl, tc, tl, sig);
1035-
__end_current_label_crit_section(cl);
1050+
__end_current_label_crit_section(cl, needput);
10361051
}
10371052
aa_put_label(tl);
10381053
put_cred(tc);
@@ -1133,10 +1148,11 @@ static int apparmor_unix_stream_connect(struct sock *sk, struct sock *peer_sk,
11331148
struct aa_sk_ctx *new_ctx = aa_sock(newsk);
11341149
struct aa_label *label;
11351150
int error;
1151+
bool needput;
11361152

1137-
label = __begin_current_label_crit_section();
1153+
label = __begin_current_label_crit_section(&needput);
11381154
error = unix_connect_perm(current_cred(), label, sk, peer_sk);
1139-
__end_current_label_crit_section(label);
1155+
__end_current_label_crit_section(label, needput);
11401156

11411157
if (error)
11421158
return error;
@@ -1163,16 +1179,17 @@ static int apparmor_unix_may_send(struct socket *sock, struct socket *peer)
11631179
struct aa_sk_ctx *peer_ctx = aa_sock(peer->sk);
11641180
struct aa_label *label;
11651181
int error;
1182+
bool needput;
11661183

1167-
label = __begin_current_label_crit_section();
1184+
label = __begin_current_label_crit_section(&needput);
11681185
error = xcheck(aa_unix_peer_perm(current_cred(),
11691186
label, OP_SENDMSG, AA_MAY_SEND,
11701187
sock->sk, peer->sk, NULL),
11711188
aa_unix_peer_perm(peer->file ? peer->file->f_cred : NULL,
11721189
peer_ctx->label, OP_SENDMSG,
11731190
AA_MAY_RECEIVE,
11741191
peer->sk, sock->sk, label));
1175-
__end_current_label_crit_section(label);
1192+
__end_current_label_crit_section(label, needput);
11761193

11771194
return error;
11781195
}

‎security/apparmor/policy.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -870,23 +870,23 @@ bool aa_policy_admin_capable(const struct cred *subj_cred,
870870
bool aa_current_policy_view_capable(struct aa_ns *ns)
871871
{
872872
struct aa_label *label;
873-
bool res;
873+
bool needput, res;
874874

875-
label = __begin_current_label_crit_section();
875+
label = __begin_current_label_crit_section(&needput);
876876
res = aa_policy_view_capable(current_cred(), label, ns);
877-
__end_current_label_crit_section(label);
877+
__end_current_label_crit_section(label, needput);
878878

879879
return res;
880880
}
881881

882882
bool aa_current_policy_admin_capable(struct aa_ns *ns)
883883
{
884884
struct aa_label *label;
885-
bool res;
885+
bool needput, res;
886886

887-
label = __begin_current_label_crit_section();
887+
label = __begin_current_label_crit_section(&needput);
888888
res = aa_policy_admin_capable(current_cred(), label, ns);
889-
__end_current_label_crit_section(label);
889+
__end_current_label_crit_section(label, needput);
890890

891891
return res;
892892
}

0 commit comments

Comments
 (0)